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

feat(web-workers): Add rollup-plugin-web-worker-loader #757

Closed
wants to merge 7 commits into from
Closed

feat(web-workers): Add rollup-plugin-web-worker-loader #757

wants to merge 7 commits into from

Conversation

schlusslicht
Copy link

@schlusslicht schlusslicht commented Nov 24, 2020

What this pull request does
This pull request aims at providing a convenient way to create inlined JavaScript source blobs which contain all dependencies to run within a WebWorker execution context.

What this pull request contains

  • implementation (here)
  • worker-loader cli option (here)
  • TypeScript-enabled test case (here)
  • small documentation change/example (here)
  • repository to test the implemented feature "in the wild" (here, npm run microbundle)

Further notes
I've tried to adhere to the standards of the codebase and the resulting changes can successfully build the rollup-typescript-webworkers repository, which is an example how to use the rollup-plugin-web-worker-loader. Feel free to point me in the right direction if You consider to merge this, but need some changes to be done! :)

Issues/drawbacks
In order to successfully use the rollup-plugin-web-worker-loader, I had to disable the Rollup build cache when running microbundle with the plugin enabled (see here). This seems to be related to an incompatibility between this plugin and something else. I've tried to investigate further, but dropped the lead after spending to much time digging through the Rollup internals. As for now, the behavior of microbundle/Rollup build cache does not change, when building without this feature.

@changeset-bot
Copy link

changeset-bot bot commented Nov 24, 2020

⚠️ No Changeset found

Latest commit: b5c2034

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@developit
Copy link
Owner

Thanks for the comprehensive PR. I'm a fan of using workers and making worker usage easier. That said, I'm not sure a prefix is the right direction for Microbundle to take. It's specific to Worker, which means we'd need to add plugins if we ever wanted to provide the identical functionality for worklets, SharedWorker, worker_threads, frames, etc.

A syntax detection-based approach like that of rollup-plugin-off-main-thread or rollup-plugin-worker would get around this concern. Ultimately, I think it might make more sense to land support for the general-purpose import workerURL from 'bundle:./worker.js' import prefix, since that can be easily used to instantiate workers and worklets.

Thoughts?

@developit developit added the increased scope Increases project scope, or is out of scope. label Dec 18, 2020
@schlusslicht
Copy link
Author

Thank You for Your review and feedback, Jason! I'm currently engaged in other projects, but will (re)evaluate this PR/funcionality some time in the future. Unless someone is faster 😃. Would like to give You an ETA, but my timeline is too messy currently.

@TimDaub TimDaub mentioned this pull request Mar 4, 2021
Targets enhancement: #170
Enable usage of `microbundle --worker-loader` to apply rollup-plugin-web-worker-loader.
and set `external: []` in the plugin options to instruct rollup to inline all external dependencies to the worker blob.
Inline sourcemaps into worker blobs only if sourcemaps are enabled (default) and compression is disabled (non-default).
@developit
Copy link
Owner

This landed in #815 / #867.

@developit developit closed this Oct 6, 2021
@schlusslicht
Copy link
Author

Hello! Yeah, I did follow up on these events recently. But as I needed this specific feature/plugin (to bundle workes inline within my library), I went ahead and monkey-patched microbundle before usage within my project ... 🙈

.. sorry that I was of no further assistance when it came to implementing/merging the alternative implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
increased scope Increases project scope, or is out of scope.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants