Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix browserify to support Object spread/rest across all files #5152

Closed
brendankenny opened this issue May 7, 2018 · 2 comments · Fixed by #5377
Closed

Fix browserify to support Object spread/rest across all files #5152

brendankenny opened this issue May 7, 2018 · 2 comments · Fixed by #5377

Comments

@brendankenny
Copy link
Member

Uncovered that you can do object spread in some of our files but not all. In the bad files, compiling gulp browserify-lighthouse in lighthouse-extension/ fails with Unexpected token. Unsure of what triggers the error in some files but not others.

To repro, just put something like const whatever = {a: 5, ...passContext}; in the afterPass of one of the problematic gatherers (like optimized-images.js).

#5129 (comment) for context:

Yet another browserify syntax error. It's on object spread again, this time in lexical-scope, called by insert-module-globals.

It appears it might be an actual bug, though, and not another parser version that needs to be piped through because you can actually use object spread in some files and not others. e.g. not in optimized-images or accessibility gatherers, but it's fine in the websql or css-usage gatherers. I haven't figured out what the error cases have in common yet or how to fix them.

@midzer
Copy link
Contributor

midzer commented May 8, 2018

Hi,
I could reproduce the bug and dig into the issue for a while.

Somehow it breaks in websql.js, too if I put

const fs = require('fs');
const axeLibSource = fs.readFileSync(require.resolve('axe-core/axe.min.js'), 'utf8');

right after Driver is required. Maybe this is of any help.

Cheers
midzer

@patrickhulce
Copy link
Collaborator

So the issue is that insert-module-globals uses lexical-scope which uses astw without options, which forces the usage of ES8/ES2017.

Simplest thing might be to do a local patch of astw since we'd need to fix the chain of lexical-scope, astw, insert-module-globals and publish new versions of each.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants