-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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: support generating multiple difference versions of worker files e.g. iife and esm #8466
Comments
According to the docs, Vite supports transpiling module worker to classic worker. But it seems it is not working. It looks like a bug. |
@sapphi-red That isn't the problem. I want to bundle the worker as both esm and iife |
@okikio would you expand a bit on how do you plan to use both workers? I asked in the linked PR but I'm interested in seeing if this feature could be implemented without the user needing to manually set the worker format but by generating both formats and automatically chose the correct one depending on the browser |
I create an empty module worker, and then test to see if the module worker crashes on the browser, if it does I switch to iife mode otherwise I use esm.
In a theoretical situation, if vite supported this use case, it would work similarly to the way the worker: {
format: ["es", "iife"],
fileName: (filename, format) => `${filename}.worker.${format == "es" ? "mjs" : "js"}`
} I would be able to easily switch between both esm and iife because I would already have the defined the file names for workers, so it would just be a simple filename extension replace. Here is an example of how it would look in real life, import {
SharedWorkerPolyfill as WebWorker,
SharedWorkerSupported,
} from "@okikio/sharedworker";
import EMPTY_WORKER_URL from "../workers/empty.js?url";
// https://stackoverflow.com/questions/62954570/javascript-feature-detect-module-support-for-web-workers
export const ModuleWorkerTest = () => {
let support = false;
const test = {
get type() {
support = true;
return "module";
},
};
try {
// We use "blob://" as url to avoid an useless network request.
// This will either throw in Chrome
// either fire an error event in Firefox
// which is perfect since
// we don't need the worker to actually start,
// checking for the type of the script is done before trying to load it.
// @ts-ignore
// `data:application/javascript;base64,${Buffer.from("export {};").toString('base64')}`
// const worker = new Worker(`blob://`, test);
// Use an empty worker file, to avoid
const worker = new Worker(
EMPTY_WORKER_URL,
test as WorkerOptions
);
worker?.terminate();
return support;
} catch (e) {
console.log(e);
}
return false;
};
export let ModuleWorkerSupported = ModuleWorkerTest();
export const WorkerType: WorkerOptions["type"] = ModuleWorkerSupported
? "module"
: "classic";
export const WorkerConfig = (url: URL | string, name?: WorkerOptions["name"]) => {
return [url.toString().replace(/\.js$/, WorkerType == "module" ? ".mjs" : ".js"), {
name,
type: WorkerType
}] as [string | URL, WorkerOptions];
};
console.log(
`This browser supports ${WorkerType} workers!`
);
console.log(
`This browser supports ${
SharedWorkerSupported ? "Shared Web Workers" : "Normal Web Workers"
}!`
);
export { WebWorker };
export default WebWorker; @patak-dev Does this help? |
Yes, thanks @okikio. @poyoho what I was trying to say in the linked PR, is to do what @okikio will do manually here, but directly as an option in core. Kind of like a plugin-legacy for workers. In plugin legacy we build both for modern browsers and for legacy ones and we switch depending on their support for a baseline (the import() trick we do). And here it will be similar, we'll test support for module workers and use them if they are available, or fallback to iife if not. I wonder if it is actually not a better default for Vite at this point, but maybe it is too big of a change. So it would be something like:
And the new |
I don't think such a complicated approach is needed for this issue. IIUC @okikio is requesting for (correct me if I'm wrong):
So I think fixing this bug (#8466 (comment), I will create a PR later) will solve it. If this bug was fixed, it is possible by using Also AFAIK there isn't much advantage of module workers. The biggest advantages of them are:
|
#8466 (comment) this seems to be fixed somewhere between 3.0.0-alpha.1 and 3.0.0-alpha.2. |
@sapphi-red You've hit some of the key points I'd like addressed, however, the current way vite supports iife is super buggy, and I'm not sure if going all in on iife is a good idea, since, the only browser that doesn't support module workers is Firefox. Also for really large projects the benefit of having module workers and being able to reuse modules in both the worker and the main thread are vital, as there is a large and noticeable boost in load time. |
Would you expand this a bit more? BTW for now (3.0.0-alpha.10), generating both classic and module worker can be done like this with import FooClassicWorker from './worker/foo.js?worker';
const fooClassicWorker = new FooClassicWorker();
const fooModuleWorker = new Worker(
new URL('./worker/foo.js', import.meta.url),
{ type: 'module' }
); though it is a strange code. I think #8466 (comment) is a nice one to have. But until chunk sharing between normal script and worker is implemented, I don't think there is a reason to generate both module and classic worker. |
I think if one entry file (no dynamic import and no external module) after rollup bundle, it will also output one file and no static import syntax? Can we disable the https://rollupjs.org/guide/en/#outputformat if the user points to the external module, I think we just need to note the effect in the document. It will generate If the user uses the dynamic import in worker, we need to tell the user the syntax only chrome supports, dynamic import is different from the static import. It seems that the worker is always The built code can run in classic and module worker too, but the results compiled by dynamic import syntax will only run in chrome 80's module worker( https://developer.mozilla.org/en-US/docs/Web/API/Worker/Worker ) correct me if I'm wrong 😁 |
@sapphi-red What I mean is that even if you build a worker using iife, vite has a tendency of mixing and matching esm imports with iife import scripts, thus the worker will not actually work. |
@sapphi-red @poyoho @patak-dev Separate but related to this issue. I keep getting this error when trying to run Astro in build mode. From what I can see this is a vite specific build issue, as shown in #6761 error Invalid value "iife" for option "output.format" - UMD and IIFE output formats are not supported for code-splitting builds.
File:
/workspaces/bundle/packages/website/src/scripts/workers/other-ts.ts?worker
Stacktrace:
at error (/workspaces/bundle/node_modules/.pnpm/rollup@2.75.6/node_modules/rollup/dist/shared/rollup.js:198:30) The problem is that I've already switched vite's worker format from I think fundamentally |
@sapphi-red @patak-dev Is there any progress? Also, @sapphi-red SSR mode doesn't support |
@okikio Were you able to figure this out? I'm kinda stuck here too. |
@micahscopes Nope. I'm still waiting for vite to support multiple workers |
ESM workers for Firefox was merged: https://bugzilla.mozilla.org/show_bug.cgi?id=1247687 Firefox 111 will be released on 2023-03-14 |
@benmccann YESSSSSSSSSSSSSSS!!!!!!!!!! hooray-letsgo.mp4 |
Thanks so much @benmccann for letting me know |
^ cc @micahscopes |
Clear and concise description of the problem
Due to size of my project, I generate multiple large worker files, but vite currently only supports generating worker files in either iife mode or esm mode.
The problem is that using esm from just a treeshake perspective, drastically improves perfs. but using esm means I can't support Firefox, and using iife means a noticeable delay in load times.
Suggested solution
Alternative
Additional context
I am building an online bundler using
esbuild-wasm
,monaco
, a bunch of fairly large plugin logic for esbuild, a couple instances of typescript, an instance ofrollup
, etc... All these files mean the worker file can ballon quite a bit very quickly.Validations
The text was updated successfully, but these errors were encountered: