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

Error when using dynamic imports #43

Closed FTWinston closed 5 years ago

FTWinston commented 5 years ago

If I include the following in my worker code (assuming dummyModule.js just contains export default 1)

const unusedImport = () => import('./dummyModule')

then I get this error:

./src/myWorker.ts (./node_modules/worker-plugin/dist/loader.js?{"name":"0"}!./src/myWorker.ts) ChunkRenderError: Conflict: Multiple chunks emit assets to the same filename static/js/0.chunk.worker.js (chunks 0 and 0)

Is this something we can work around with worker-plugin's options?

FTWinston commented 5 years ago

I can work around this error by specifying a name in the worker's options:

new Worker('./myWorker.ts', { name: 'mine', type: 'module' });

I'd consider this to be a good-enough workaround.

qidaneix commented 5 years ago

hi bro, will your eslint throw error when you use ts with worker-plugin? mine did, and donot know why

dragly commented 4 years ago

Thanks for the workaround. Just a heads-up to anyone who tries the same:

Make sure the type comes after the name. Otherwise it will break because the type is removed from the code and a comma is left behind:

new Worker('./myWorker.ts', { type: 'module', name: 'mine' });

becomes

new Worker('./myWorker.ts', { , name: 'mine' });

This will cause bundlers to fail when using the code.

art-in commented 4 years ago

This shouldn't be closed since workaround is not a real fix.

gluck commented 4 years ago

Bit by this issue as well, I was providing a name but a non-constant one, so plugin would still go for chunkname === '0'. I believe this is caused because '0' chunk name (provided to SingleEntryPlugin) could eventually conflict with webpack ID-based chunk names (if you do dynamic imports or chunk splitting).

Easy fix confirmed is to use something else like 'W' + workerId as default name argument to loader.js.

Happy to PR this 1 line change if anyone with better webpack skills than mine can confirm it's the right :tm: way to go.

developit commented 4 years ago

@gluck that actually seems like a pretty good solution. Definitely open to a PR, we can see if it still passes the tests.