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

import statement in Workers #4586

Closed
7 tasks done
Alexufo opened this issue Aug 12, 2021 · 14 comments · Fixed by #7099
Closed
7 tasks done

import statement in Workers #4586

Alexufo opened this issue Aug 12, 2021 · 14 comments · Fixed by #7099
Labels
feat: web workers p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@Alexufo
Copy link

Alexufo commented Aug 12, 2021

Describe the bug

Due Firefox limit to support import statement in a web workers, how we can create production bundle?

I get "SyntaxError: import declarations may only appear at top level of a module" in FF.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import

Reproduction

use import {myExport} from '/modules/my-module.js'; inside worker.

System Info

Windows, nightly  FF 93.0a1

Used Package Manager

npm

Logs

No response

Validations

@mathe42
Copy link
Contributor

mathe42 commented Aug 25, 2021

To use workers in development you have to use a suported browser (see https://caniuse.com/mdn-api_worker_worker_ecmascript_modules).

In production the worker is bundled in a new conext with no import statements.

Just as a refrence here the bug in the Firefox bugtracker for module Worker https://bugzilla.mozilla.org/show_bug.cgi?id=1247687.

@Alexufo
Copy link
Author

Alexufo commented Aug 25, 2021

@mathe42 Yes, it must inline es modules in bundle. Is it possible now with rollupjs setting?

@mathe42
Copy link
Contributor

mathe42 commented Aug 25, 2021

In production (and current vite version) worker work in firefox.

@Alexufo
Copy link
Author

Alexufo commented Aug 25, 2021

Yes. But we cant use es modules inside worker in FF now. I dont find a way how inline es modules inside worker for npm run build

@mathe42
Copy link
Contributor

mathe42 commented Aug 25, 2021

It is done automaticly no options required. Just build it and then open result in FF.

@Alexufo
Copy link
Author

Alexufo commented Aug 26, 2021

I dont see this behavior.Please check MVP.
npm run build ignore all files in binding folder
https://github.com/Alexufo/test_vite/

npm run dev and you will see in console "hello world" from wasm. It is working only in dev mode.

@mathe42
Copy link
Contributor

mathe42 commented Aug 26, 2021

Oh I see the problem. Should be easy to fix...

@mathe42
Copy link
Contributor

mathe42 commented Aug 26, 2021

Created a PR (#4741) is a simple mistake 😂.
You can try my comlink plugin wich doesn't has this bug 😉
https://github.com/mathe42/vite-plugin-comlink

@Alexufo
Copy link
Author

Alexufo commented Aug 26, 2021

@mathe42 thanks, You should understand - it is a unbelievable problem if you are just install vite yesterday :-)

@mathe42
Copy link
Contributor

mathe42 commented Aug 27, 2021

I had another look at your code you don't import a Worker! This is why it dosn't work!

Use

import MyWorker from './worker-file?worker'

const worker = new MyWorker()

The problem in my PR doesn't fix your problem. Use the above and in production it works in FF.

@Alexufo
Copy link
Author

Alexufo commented Aug 27, 2021

Thank you! Yes it working. But

  1. Why Vite don't recognize native construction?
    const SEWorker = new Worker('./bindings/file.js', { type: 'module' });
    I don't think it is a normal include worker as dependency via import only.

  2. Vite don't recognize dependencies from ./bindings/file.js from emscripten. There is no wasm file included in dist folder. Rewriting ./bindings/file.js module from emsctripten not a good idea.

Can it be solved if we include deps via https://vitejs.dev/config/#dep-optimization-options ?

@mathe42
Copy link
Contributor

mathe42 commented Aug 27, 2021

Vite doesn't parse your file where ever posible. So there is no way to detect a constructor call (or wasm calls).

You can write your own vite plugin that can does that (not that complicated)

Quick idea for Worker:

in tranform search code for new Worker(...) (with regex) than mopdify source code (add import on top; change Worker creation).

Should be a plugin with 20 Lines of code or so. But it has edge cases where it will transform to much. And there is no way to do that properly on the fly.

Tip for emscripten glue-code:

Put the wasm file in the public folder this way vite don't need to handle that file. You might want to check if the glue code expects some subfolders.


For workers I think comlink is so nice that I want to have the custom imports for type checking. (I use allways Typescript.)

@Alexufo
Copy link
Author

Alexufo commented Sep 13, 2021

@mathe42 thank you. Can I automate coping wasm file to asset folder? Vite does not discover wasm files. Or better to do it in plugin?

@Filyus
Copy link

Filyus commented Jan 25, 2022

This thing makes it harder to test in Firefox, it would be nice to be able to turn it off:
code: `import '${ENV_PUBLIC_PATH}'\n` + _

@g-plane g-plane added p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Feb 18, 2022
@g-plane g-plane added the has pr label Feb 18, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: web workers p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
5 participants