-
Notifications
You must be signed in to change notification settings - Fork 200
Threads.js Bundling #150
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
Comments
Hi @ilan-gold With the previous "solution", i.e webpack, we had a very similar problem. It required users to use the "worker-loader" plugin with a weird configuration to work. The underlying issue is that WebWorkers are weird to work with, especially with bundling tools. I'm sorry this is now causing issues for you. Currently I have no clear and easy remedy, nor a roadmap to fix this. |
FWIW @constantinius I never had that issue in the browser (having to bundle with |
Hi. I'm by no means an expert in web workers, threads, etc., but wanted to share some notes on how we got the new version of geotiff.js to work with GeoRaster which uses Webpack. We run a script to replace the parcel-friendly code in the geotiff.js source code In our webpack.config.js we use the ThreadsPlugin, set
I couldn't agree more! :-) Perhaps we could work together on a library that solves these issues at a more fundamental level? Are there any examples of libraries that try to bring threads and worker-loader together similar to how cross-fetch made it easier to use whatwg-fetch and node-fetch? Happy to pitch in and open to your ideas :-) |
Hey, I agree that web worker are difficult to bundle and I currently don't know what is the best option. I wasn't pleased to introduce the ThreadPugin dependency 😕, I agree that this is not ideal. I think you only need the ThreadPlugin if you want to build from source, if you import the prebuilt worker in the dist-node or dist-browser you don't have to rebuild the worker with ThreadPlugin (it would require some tests to confirm that this is true). For example building a NodeJS application should not require the ThreadPlugin import. But most of the commonly used frontend framework (React, VueJS..) use webpack and target the Maybe we could address this issue by providing a 3rd bundle that compile the module in ESM format and apply the changes done by the ThreadPlugin, and therefore avoid the upstream dependency and also prevent collision with the client if he uses raw WebWorkers or an other library that overwrite the Worker constructor like threads.js. But parcel is too limited to tweak the build in that way, so we should probably try to do these builds with webpack or rollup. (https://github.com/purtuga/esm-webpack-plugin)
I think this workaround is not necessary with the latest version :D (#145, #144). Could you confirm ? |
@PacoDu This was not my experience with threads.js - I needed to have the plug-in serve the worker file out of my |
I wonder if an option in this space is this library - https://github.com/developit/web-worker |
@ilan-gold sorry for not answering earlier, If I recall correctly the worker was simply not compatible with NodeJS @rowanwins thanks for pointing to this library ! I took a quick look and I see a really interesting point in the documentation of web-worker:
I think this could solve our issue of serving the web-worker to frontend development servers. And if this is compatible with threadjs we could keep the nice interface that threadjs offers. Or switch to web-worker if it doesn't work with threadjs |
No worries. That package looks very interesting. I had no idea about the Node usage. Still, for the browser, I think we'd want to do inline bundling since upstream applications would not want to worry about dealing with that. If this handles that cleanly, though, even without inline bundling, then I think this is a good fix. FWIW I think that keeping the |
Hey there! I don't know if you saw it already or if it's still an urgent topic for you, but threads.js now supports inlining worker code using BlobWorker, so you can ship master and worker code together in a single module. Only gotcha right now: When bundling you need to run webpack twice. First bundle the worker(s) and then run webpack again for the main entrypoint code. |
Hey, @andywer thanks ! |
Thanks, much appreciated |
I have been thinking more about this and I realized that I don't want to ask upstream-users of my library to bundle the workers from threads.js via webpack or parcel. It seems that there isn't an accepted way to write a "library" using this (see andywer/threads.js#211 and andywer/threads.js#232 (comment)) since you can't get a single bundle easily. If I have this wrong, I'd love to know how to get this to work, but it seems that there isn't an easy way to publish a library that uses threads.js without having upstream users handle the implicit code-splitting. Again, if I have this wrong, please let me know - despite the bundling, threads.js is far easier to understand/use!
I reverted to using the old web workers from an old release of GeoTIFF in my application: hms-dbmi/viv#160. In my testing via
npm pack
I don't seem to have any issues with the bundling and it works well. I understand the ease of parcel as well as threads.js as opposed to using webpack/webworkers but I just wanted to leave this here for anyone else who might encounter this, or if changes are desired, to help start a roadmap for them.CC: @PacoDu @constantinius
The text was updated successfully, but these errors were encountered: