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

Use simple import.meta.url for worker creation #23889

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JoeOsborn
Copy link
Contributor

@JoeOsborn JoeOsborn commented Mar 11, 2025

This needs to be tested under various bundlers. Spun out from #23804 . Allows for scripts to be renamed after compilation.

Vite and (unreleased) Webpack and (I believe) Parcel allow for the form new Worker(import.meta.url), which simplifies the new worker creation in libpthread and avoids the need for using a macro to expand the current script name (which makes the generated code fragile).

I’ve taken the opportunity to expand the test coverage for permutations of bundlers and mainScriptUrlOrBlob with and without threads (qua #23890 ), but if this feels redundant then the change in libpthread is enough.

This needs to be tested under various bundlers.
The webpack tests won't pass until the next release, but I have
checked that they do pass under webpack's latest git HEAD.

I also have failing tests here for vite and webpack under the
mainScriptUrlOrBlob condition.  These bundlers do funny things with
paths and modules so I think it's worth having the coverage.
@kleisauke
Copy link
Collaborator

Looking at the test failures in test-other, I think it needs to be new Worker(new URL(import.meta.url)), at least on Node.js.

TypeError [ERR_WORKER_PATH]: The worker script or module filename must be an absolute path or a relative path starting with './' or '../'. Wrap file:// URLs with `new URL`. Received "file:///tmp/emtest_zpd2a426/emscripten_test_other_ybmo_i00/subdir/module.mjs"

We could perhaps use new Worker(import.meta.url) when ENVIRONMENT_MAY_BE_WEB && !ENVIRONMENT_MAY_BE_NODE, but this code section already has quite a few conditional expressions. 😅

@sbc100
Copy link
Collaborator

sbc100 commented Mar 15, 2025

Looking at the test failures in test-other, I think it needs to be new Worker(new URL(import.meta.url)), at least on Node.js.

TypeError [ERR_WORKER_PATH]: The worker script or module filename must be an absolute path or a relative path starting with './' or '../'. Wrap file:// URLs with `new URL`. Received "file:///tmp/emtest_zpd2a426/emscripten_test_other_ybmo_i00/subdir/module.mjs"

We could perhaps use new Worker(import.meta.url) when ENVIRONMENT_MAY_BE_WEB && !ENVIRONMENT_MAY_BE_NODE, but this code section already has quite a few conditional expressions. 😅

Or we could improve our new Worker polyfill on node so that it support string as a well as URLs.

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

3 participants