GoogleChromeLabs / worker-plugin

👩‍🏭 Adds native Web Worker bundling support to Webpack.
https://npm.im/worker-plugin
Apache License 2.0
1.92k stars 79 forks source link

Trailing Comma in the LoaderOption causes SyntaxError on removing `type:module` #25

Closed lacolaco closed 4 years ago

lacolaco commented 5 years ago

At first, I think this is not only by the WorkerPlugin's implementation.

Problem

When {type: 'module'} contains trailing-comma like below, compiled JavaScript is broken.

// pre-compile
new Worker('./foo.js', { 
  type: 'module',
});
// post-compile
new Worker('./foo.js', { 
  , // <--- SyntaxError
});

Workaround

Removing the comma.

 new Worker('./foo.js', { type: 'module' });

Version

I confirm reproducing in v3.0.0 and v3.1.0.

lacolaco commented 5 years ago

This can happen frequently if developers are using code formatter like Prettier.

developit commented 5 years ago

ooof! good catch.

viceice commented 5 years ago

the same is happen if you use additional options like:

new Worker('./monaco-css.worker', { type: 'module', name: 'monaco-css' });

My current Workaround is to flip type as last property.

Why preserveTypeModule is undefined by default? should not type be preserved by default?

developit commented 5 years ago

@ViceIce preserving {type:'module'} forces the worker code into strict mode by default, which is unexpected given it's being used to load CommonJS webpack output. I believe there are also environments in which { type: 'module' } throws when passed to Worker().

developit commented 5 years ago

If anyone has a deep working knowledge of Webpack's funky parser modifiers, this is the line that replaces type:'module' with an empty expression. Maybe it needs to be null? I have no idea. https://github.com/GoogleChromeLabs/worker-plugin/blob/master/src/index.js#L79