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

When using import inside classic worker, syntax error happens during dev but build success #8470

Open
7 tasks done
sapphi-red opened this issue Jun 5, 2022 · 22 comments
Open
7 tasks done
Labels

Comments

@sapphi-red
Copy link
Member

sapphi-red commented Jun 5, 2022

Describe the bug

When import is used in classic worker, the following error happens during dev.

Uncaught SyntaxError: Cannot use import statement outside a module

But build works with it and the bundle seems to be correctly bundled.

According to the docs, import can be used inside classic worker if worker is imported with query suffixes.
There is no mention about constructor type worker import. But if it is the recommended way, I think it would be good to support it too. Also it is useful for #8466.

#8466 (comment) might be related.

Reproduction

https://stackblitz.com/edit/vitejs-vite-mhmcjy?file=main.js

System Info

stackblitz

Vite: 2.9.9, 3.0.0-alpha.9

Used Package Manager

npm

Logs

No response

Validations

@poyoho
Copy link
Member

poyoho commented Jun 6, 2022

@sapphi-red I think we just need to support

(case 1)

import 'a.js'

instead of

(case 2)

import('a.js')

IIUC, importScript is async and case 1 is also async. 😁

https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope/importScripts

@poyoho
Copy link
Member

poyoho commented Jun 6, 2022

oh, but the import syntax is native support

@sapphi-red
Copy link
Member Author

Just in case, importScripts is sync.

Yep, import is supported inside module worker. But it does not work inside classic worker without any transforms.

@poyoho
Copy link
Member

poyoho commented Jun 6, 2022

I test import in dev is work well. It seems to have native support from classic workers too.

@sapphi-red
Copy link
Member Author

sapphi-red commented Jun 6, 2022

I don't think it will work and it is actually not working in my reproduction.

My understanding is:

import dev build
import w from 'foo.js?worker' module worker.format
new Worker(new URL('foo.js', import.meta.url)) classic classic
new Worker(new URL('foo.js', import.meta.url), { type: 'module' }) module module

So it does not work only if it is imported with new Worker(new URL('foo.js', import.meta.url)).

@poyoho
Copy link
Member

poyoho commented Jun 6, 2022

import w from 'foo.js?worker' build time is also module.

new Worker(new URL('foo.js', import.meta.url)) is support import syntax in dev(I used chrome browser). But rollup format: iife will got an error.

worker.js

import('./modules/module2.js').then(module0 => {
  self.postMessage({
    type: 'classic-worker-import',
    content: module0.default
  })
})

./modules/module2.js

export * from './module0'
export * from './module1'
export const msg2 = 'module2'

It seems './modules/module0.js' run as <script type="module"> in classic worker.

@sapphi-red
Copy link
Member Author

dynamic import are supported in both classic and module worker.
But static import are not supported in classic worker.

import w from 'foo.js?worker' build time is also module.

It is classic if worker.format is iife (stackblitz 3.0.0-alpha.9, though it is es when vite 2.9.9).

It seems './modules/module0.js' run as <script type="module"> in classic worker.

Files imported by static or dynamic import is treated as ESM (even if they are import from a classic worker/script).

@poyoho
Copy link
Member

poyoho commented Jun 6, 2022

so the issue asks for

  1. dynamic import support in classic worker (build mode)
  2. static import support in classic worker (dev mode)
    😵

@sapphi-red
Copy link
Member Author

sapphi-red commented Jun 6, 2022

To be honest, I didn't know that dynamic import works inside classic worker/script. At the time I created this issue, I was only thinking about static imports.

Also, now I think supporting static import implicitly in classic worker is confusing as it misaligns with the spec.
I come up with this notation but there is no spec about this too.

new Worker(new URL('./foo.js', import.meta.url), { type: 'classic', as: 'module' })

Maybe we could just stop static import working in classic worker with build?

@poyoho
Copy link
Member

poyoho commented Jun 6, 2022

If support static import working in classic worker, I think we can transform code like this

Promise.all([
  import('./a.js'),
  import('./b.js'),
]).then(mods => {
  const [
    moda,
    modb
  ] = mods
  .......
})

I think this is acceptable. but dynamic import support in classic worker it seems can't mock. because we can't make dynamic import deps to load on demand, we just can make them to static import and handle it.

if(flag) {
  import('./foo.js').then((mods) => {
    self.postMessage(mods)
  })
}

tobe

import __vite_worker_dynamic_import_0 from './foo.js'
if(flag) {
  Promise.resolve({default: __vite_worker_dynamic_import_0}).then((mods) => {
    self.postMessage(mods)
  })
}

and transform by rollup

BTW, import worker from "./worker.js?worker" is make all the workers be module workers in dev mode. Can we make the worker constructor new a module worker too?

To stop static import working in classic worker with the build is okay too 😂

@sapphi-red
Copy link
Member Author

Can we make the worker constructor new a module worker too?

My first thought was this one. But the notation/syntax needs to be considered.

@poyoho
Copy link
Member

poyoho commented Jun 7, 2022

I think the better way is to compile it but it will use more performance. I think now better to alert document

@egriff38
Copy link

Following this issue. This is the biggest inhibitor to running workers with imports in dev mode with Firefox. I can't be the only one who develops against Firefox, right?

Banou26 added a commit to Banou26/libav-wasm that referenced this issue Dec 14, 2022
@deluksic
Copy link

deluksic commented Jan 9, 2023

Some colleagues prefer Firefox for development and we would like to debug on FF as well.

This issue prevents development on Firefox for any project that uses Workers with imports.

Is there a way to tell vite to inline imports into the worker file during development?

@eliot-akira
Copy link

eliot-akira commented May 13, 2023

Looks like Firefox 114 will support import within workers.


EDIT: Confirmed.

FF114 adds support for ECMAScript modules. What that means is that you can use the constructor {type: "module"} in worker/shared worker constructor, and use both import and import(). In Worklets you can call import.
FF does not yet support modules in ServiceWorkers, but they also allow import only.

FF114 ECMAScript module support

@bluwy
Copy link
Member

bluwy commented Nov 10, 2023

Related: #10697 (feature request to always bundle workers)

@yvesgurcan
Copy link

Hello! I just ran into this issue and spent several hours trying to understand why the service worker of my application was throwing an error.

Now that Firefox apparently supports ECMAScript modules for the Worker() constructor, what would it take for Vite PWA to use the module type and enable the development of service workers on Firefox?

@eliot-akira
Copy link

eliot-akira commented Feb 9, 2024

Firefox support for module import in service workers is still being implemented. It's tracked here:

@yvesgurcan
Copy link

Thank you for the link!

@itsjohncs
Copy link

I was running into this as well so I made a plugin to implement the functionality: https://github.com/itsjohncs/vite-plugin-prebundle-workers. But I also think this would make sense to do by default in Vite.

@dariusve
Copy link

I was running into this as well so I made a plugin to implement the functionality: https://github.com/itsjohncs/vite-plugin-prebundle-workers. But I also think this would make sense to do by default in Vite.

I tried to use and I still get the "Cannot use import statement outside a module" error when the worker tries to import some modules

@itsjohncs
Copy link

I was running into this as well so I made a plugin to implement the functionality: https://github.com/itsjohncs/vite-plugin-prebundle-workers. But I also think this would make sense to do by default in Vite.

I tried to use and I still get the "Cannot use import statement outside a module" error when the worker tries to import some modules

@dariusve open up an issue in itsjohncs/vite-plugin-prebundle-workers with some more info (configuration and code that's failing) and I'll try and help you debug.

My guess is the plugin isn't configured to bundle the right file(s), but there may be something else going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants