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

sapphi-red commented 2 years ago

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](https://vitejs.dev/guide/features.html#:~:text=The%20worker%20script%20can%20also%20use%20import%20statements%20instead%20of%20importScripts()) 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.

https://github.com/vitejs/vite/issues/8466#issuecomment-1146655205 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

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/core instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

poyoho commented 2 years ago

@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 commented 2 years ago

oh, but the import syntax is native support

sapphi-red commented 2 years ago

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 commented 2 years ago

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

sapphi-red commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

so the issue asks for

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

sapphi-red commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

egriff38 commented 1 year ago

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?

deluksic commented 1 year ago

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 commented 1 year ago

Looks like Firefox 114 will support import within workers.

Ship module workers https://bugzilla.mozilla.org/show_bug.cgi?id=1812591

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 commented 10 months ago

Related: https://github.com/vitejs/vite/issues/10697 (feature request to always bundle workers)

yvesgurcan commented 7 months ago

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 commented 7 months ago

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

Implement "module" service workers

yvesgurcan commented 7 months ago

Thank you for the link!

itsjohncs commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

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.

vitejs / vite