worker: add support for Web Workers - #64894
Conversation
|
Review requested:
|
|
I'm not sure if I should break the WPT additions into their own commit / PR for ease of reviewing? You should be able to collapse the Finally, I haven't added dedicated tests for this outside of the WPT, which should cover it. |
|
The web platform tests in this PR really ought to be separated out into a separate commit to make reviewing this easier. 1300+ files changes with 54k+ lines changed is very difficult to review in a single commit. |
Broken up! c73f2fe has the actual changes |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #64894 +/- ##
==========================================
- Coverage 90.32% 90.13% -0.19%
==========================================
Files 751 752 +1
Lines 250000 251094 +1094
Branches 47231 47157 -74
==========================================
+ Hits 225816 226336 +520
- Misses 15566 16131 +565
- Partials 8618 8627 +9
🚀 New features to boost your workflow:
|
|
This is gargantuan. Can you add a review guide and/or split into chunks? |
|
I have:
|
It's only extremely large due to the added WPT tests. You can make it easier to review by
|
|
I think I'm getting really really unlucky on the MacOS CI. It's failing over several attempts, but each time with a different test flake 😭 |
|
For easier review of the PR |
|
@nodejs/web-standards @nodejs/workers PTAL :-) |
|
I simulated the daily Node.js WPT workflow against
Regression coverage was added for both discovery filtering and end-to-end window/worker title parity. |
Commit Queue failed- Loading data for nodejs/node/pull/64894
✔ Done loading data for nodejs/node/pull/64894
----------------------------------- PR info ------------------------------------
Title worker: add support for Web Workers (#64894)
Author Aviv Keller <me@aviv.sh> (@avivkeller)
Branch avivkeller:web-workers -> nodejs:main
Labels semver-minor, lib / src, experimental, author ready, worker, needs-ci, review wanted, commit-queue, commit-queue-rebase, web-standards
Commits 9
- test: accomodate multi-global tests in WPT{Runner,TestSpec,Report}
- test: add opt-in process WPT runner
- worker: add support for Web Workers
- test: enable multi-global WPTs
- worker: add wpt tests for Web Workers
- fixup! worker: add support for Web Workers
- fixup! test: enable multi-global WPTs
- fixup! worker: add wpt tests for Web Workers
- fixup! test: enable multi-global WPTs
Committers 2
- Filip Skokan <panva.ip@gmail.com>
- avivkeller <me@aviv.sh>
PR-URL: https://github.com/nodejs/node/pull/64894
Fixes: https://github.com/nodejs/node/issues/43583
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64894
Fixes: https://github.com/nodejs/node/issues/43583
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
ℹ This PR was created on Sat, 01 Aug 2026 04:18:11 GMT
✔ Approvals: 2
✔ - Filip Skokan (@panva) (TSC): https://github.com/nodejs/node/pull/64894#pullrequestreview-4868941830
✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/64894#pullrequestreview-4926739457
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2026-08-05T20:04:47Z: https://ci.nodejs.org/job/node-test-pull-request/75538/
⚠ Commits were pushed after the last Full PR CI run:
⚠ - fixup! worker: add wpt tests for Web Workers
⚠ - fixup! test: enable multi-global WPTs
- Querying data for job/node-test-pull-request/75538/
✔ Build data downloaded
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/31701830064 |
| // https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the-worker-interface | ||
| exposeLazyInterfaces(globalThis, 'internal/webworker', ['Worker']); |
There was a problem hiding this comment.
Shouldn't this be a conditional?
| function setupWebWorkers() { | ||
| if (!getOptionValue('--experimental-web-worker')) { | ||
| delete globalThis.Worker; | ||
| } | ||
| } |
There was a problem hiding this comment.
The naming is deceiving, it's not setting up anything
There was a problem hiding this comment.
I was following how setupEventsource and how setupWebsocket worked, is that wrong?
There was a problem hiding this comment.
I can change it to behave more like setupWebStorage?
There was a problem hiding this comment.
Maybe there's a good reason for doing it this way (e.g. if getOptionValue is not available, or something like that), following setupWebStorage is not a good enough reason
There was a problem hiding this comment.
Okay, so this can be resolved since I'm following the similar APIs, or is there something else to discuss?
There was a problem hiding this comment.
I'm saying the opposite (i.e. setupWebStorage is not an example to follow), though I guess it's not a blocking concern
| WritableStreamDefaultWriter: 'readonly', | ||
| WritableStreamDefaultController: 'readonly', | ||
| WebSocket: 'readonly', | ||
| Worker: 'readonly', |
There was a problem hiding this comment.
We need to also forbid its use in lib/ like we do for e.g.
node/lib/eslint.config_partial.mjs
Lines 299 to 302 in c47ad66
There was a problem hiding this comment.
We don't apply this logic to everything, so I wasn't sure (e.g. WebSocket, URLPattern). Are those intentional exceptions or just oversights?
There was a problem hiding this comment.
Just oversight, I've opened #65281 to fix that. We don't want to rely on the user-mutable global object unless we don't have a choice
Signed-off-by: Filip Skokan <panva.ip@gmail.com> Co-authored-by: Aviv Keller <me@aviv.sh> Signed-off-by: Aviv Keller <me@aviv.sh>
To hopefully get to the bottom of WPT crashes that have no traces. Signed-off-by: Filip Skokan <panva.ip@gmail.com> Co-authored-by: Aviv Keller <me@aviv.sh> Signed-off-by: Aviv Keller <me@aviv.sh>
Signed-off-by: Aviv Keller <me@aviv.sh>
Signed-off-by: Filip Skokan <panva.ip@gmail.com> Co-authored-by: Aviv Keller <me@aviv.sh> Signed-off-by: Aviv Keller <me@aviv.sh>
Signed-off-by: Aviv Keller <me@aviv.sh>
|
rebased and squashed, i made no functional changes |
Closes: #43583
Adds support for the Web Worker API as defined by the HTML Standard:
https://html.spec.whatwg.org/multipage/workers.html
The implementation trys to follow the specification as close as Node.js allows, so note the following differences:
SharedWorkeris not implemented. Its lifetime and sharing model depend on origins and browsing contexts, concepts that do not exist in Node.js.Worker scripts are loaded synchronously from the local filesystem rather than fetched over the network. As a result:
new Worker()andimportScripts()accept onlyfile:,data:, andblob:URLs.NotSupportedError.NetworkError(per the spec, this is emitted as anerrorevent).nosniff, and HTTP MIME type validation are not applicable. MIME type validation is performed only fordata:andblob:URLs.WorkerOptions.credentialsis validated for API compatibility but otherwise has no effect, since no network request is made.Node.js has no origin model. Consequently, concepts such as same-origin and cross-origin do not exist, and
location.originisnullforfile:workers.close()terminates the worker immediately instead of following the specification's "closing flag" algorithm. Code remaining in the current task afterclose()is therefore not executed.The worker global is the normal Node.js global object with
DedicatedWorkerGlobalScopeinserted into its prototype chain rather than the inverse (a fresh global created from the interface). Additionally, classicfile:workers are executed through the CommonJS/ESM loaders rather than as classic scripts, so top-level declarations do not become global properties. Classicdata:andblob:workers continue to execute as classic scripts.ErrorEvents dispatched toWorkerinstances includemessageanderror, but notfilename,lineno, orcolno. Unhandled worker errors are also not propagated further. These are a result of theworker_threadsimplementation that is underneath the web workers implemantion.The following
WorkerGlobalScopeevents are never dispatched:languagechange,online, andoffline, since these concepts do not exist in Node.js.rejectionhandledandunhandledrejection, since Node.js exposes equivalent process-level events but does not implement thePromiseRejectionEventinterface or the per-rejectionpreventDefault()behavior required by the HTML Standard.On the main thread, relative worker script URLs are resolved against the current working directory because there is no document base URL. Within a worker, relative URLs resolve against the worker's own URL, matching the specification.
AI Disclaimer: I used slight AI help to resolve issues that came up during me validating the WPT tests.
TODO before merge: