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

docs: add warning on Worker import with constructor limitations #15436

Merged
merged 8 commits into from
Jan 26, 2024

Conversation

malta895
Copy link
Contributor

@malta895 malta895 commented Dec 27, 2023

Description

The explanation on how to import Workers with Constructor is not explaining how Vite actually parses the user code at build time.

I tried to import a SharedWorker like this:

const WORKER_NAME = 'MyWorkerName'
const workerUrl = new URL('./worker.js', import.meta.url)
const worker = new SharedWorker(workerUrl, {
    type: 'module',
    name: WORKER_NAME
))

This isn't working for two reasons:

  • the name option isn't a static value, and it triggers a build error
  • workerUrl is declared in a separated variable, and this is not recognized by Vite

The latter does not even trigger an error, but just a warning. This ends up in a potentially broken build, without the Worker being bundled correctly.

Reference to the relevant code: https://github.com/vitejs/vite/blob/56ae92c33cba6c86ab5819877c19b9ea39f7121b/packages/vite/src/node/plugins/workerImportMetaUrl.ts

I added a warning that explains this behavior.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

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

  • Read the Contributing Guidelines, especially the Pull Request Guidelines.
  • 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).
  • Update the corresponding documentation if needed.
  • Ideally, include relevant tests that fail without this PR but pass with it.

Copy link

stackblitz bot commented Dec 27, 2023

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

docs/guide/features.md Outdated Show resolved Hide resolved
@malta895 malta895 marked this pull request as ready for review December 27, 2023 08:55
@malta895 malta895 marked this pull request as draft December 27, 2023 08:55
@malta895 malta895 marked this pull request as ready for review December 27, 2023 09:13
docs/guide/features.md Outdated Show resolved Hide resolved
@malta895 malta895 changed the title fix: warning on new Worker limitations fix: warning on Worker import with constructor limitations Dec 27, 2023
docs/guide/features.md Outdated Show resolved Hide resolved
docs/guide/features.md Outdated Show resolved Hide resolved
@bluwy bluwy changed the title fix: warning on Worker import with constructor limitations docs: add warning on Worker import with constructor limitations Jan 8, 2024
docs/guide/features.md Outdated Show resolved Hide resolved
Co-authored-by: patak <matias.capeletto@gmail.com>
@sapphi-red sapphi-red merged commit 3fceb14 into vitejs:main Jan 26, 2024
10 checks passed
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.

None yet

4 participants