Minified JS overestimates savings when esprima fails

[RonnieWinter]

Provide the steps to reproduce

When running lighthouse tests against our AMP pages we noticed that one of the js scripts shows up on the report as not being minified. The issue is on this file: https://cdn.ampproject.org/v0/amp-date-picker-0.1.js
This is because that file contains a copyright noticed that is hard coded into the minified javascript file, which contains carriage returns and whitespace.
I opened an issue in the AMPHTML repository asking if they could minify the copyright notice, but they said that they cannot change it and that the problem lies with Lighthouse being too picky.

For more information this is the issue I opened in the AMPHTML repository: Issue 17077

Environment Information

• Affected Channels: CLI, Node, Extension, DevTools
• Lighthouse version: 3.0.1
• Operating System: All
  

[patrickhulce]

Thanks for reporting @ro2ni3! This definitely seems like a bug. The copyright notices should definitely not be ~97% savings on the file we're saying :) We'll take a closer look

[patrickhulce]

Turns out this is a bug in esprima which we use to tokenize the JS. It seems to erroneously think the snippet from line 54 of that file (reproduced below) has an illegal token (though oddly when invoking esprima.parse it is able to handle it correctly)

if (-1 !== d.indexOf(","))
  for (d = d.split(","), k = 0; k < d.length; k++)
    /* fails on this regex here --> */ /^[+-]?\d+$/.test(d[k]) && (d[k] = Number(d[k]));

because LH uses tolerant mode, it reports what it was able to parse which is the first 3% of the file hence 97% savings :)

[paulirish]

basically esprima sucks with regex sometimes. we use tolerant because of that

options:

1. switch from esprima to acorn or something else
2. turn back off tolerant, and deal with the errors (airhorner, etc)
3. fix this upstream in esprima? :/