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: legacy build of web workers #12320

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

Tal500
Copy link
Contributor

@Tal500 Tal500 commented Mar 7, 2023

Closes #10383

Description

Enable web workers to be built correctly for legacy browsers.
The current situation is that the workers are built by themselves via the internal node plugin worker.js, ignoring any legacy output that is desired.
For fixing this issue in the ideal way, the changes should be both for plugin-legacy and for the internal node plugin worker.js.

Here is my roadmap(including explanations and more), I will be glad to hear your opinion about it:

  • Export an additional workerPlugins field from the plugin-legacy, that is responsible for transforming the rendered chunk to an actual one that is legacy-safe. Note: The user needs to manually put this result field in the Vite configuration worker.plugins. Sadly, this worker's rollup plugin cannot be registered now automatically, because Vite forbid/ignore when one plugin change the configuration for loading a different plugin in the config step of Vite plugins.
  • Add a case for a "worker-modern" in the playground, a worker that uses Promise and therefore needs to load legacy polyfills.
  • Instead of inlining the polyfills inside the workers, just add them to the global legacy/modern polyfills sets and load them dynamically in the worker via importScripts(). This will results both with a better respect for the configurations and reduced worker size. There will be no extra delay for the client, since the browser already had download the JS chunk of the legacy/modern polyfills in the script pre-initialization.
  • Since some of the polyfills in the "additionalPolyfills" configuration may assume they have access to window or document, and it's not available in web workers context, we need to have another configuration "additionalPolyfillsDocumentSensitive", which all of them will be build and injected to the big polyfill file, but with a big "if" statment that apply them if we're in a document context(i.e. not in a web worker).
  • Allow a separated build of web workers in the "worker.js" plugin per each output: This can be implemented if we move the place that the web workers are built and injected/imported-replaced in the file, from the "transform" stage to the "renderChunk" stage. Then, we can pass in some meta-config of Rollup output configuration, the object of the rendred chunk and the output configuration, so the rollup plugins that the user have passed in the worker.plugins will know more details about the specific output. This will allow the plugin-legacy to know if we're building the legacy or the modern chunk, and more info.

What is the purpose of this pull request?

  • New Feature

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@stackblitz
Copy link

stackblitz bot commented Mar 7, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@Tal500 Tal500 changed the title Legacy build of web workers feat: legacy build of web workers Mar 7, 2023
@Tal500
Copy link
Contributor Author

Tal500 commented Mar 7, 2023

@poyoho I think you'd like to read this PR description, since it seems that you're involved a lot in web workers here and legacy. Of course as anyone else, I'd like also to hear your review also.

This moves the logic of the worker build from "transform" to "renderChunk".
An issue that was introduced: The output dir now isn't `workers/`, but `assets` instead (need fixing).
@christowiz christowiz mentioned this pull request Apr 24, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow Web Workers on legacy
1 participant