fix(ext/node): run worker_threads eval code in sloppy mode#32428
Merged
bartlomieju merged 8 commits intomainfrom Mar 10, 2026
Merged
fix(ext/node): run worker_threads eval code in sloppy mode#32428bartlomieju merged 8 commits intomainfrom
bartlomieju merged 8 commits intomainfrom
Conversation
Node.js evaluates `{ eval: true }` worker code as CommonJS (sloppy mode),
but Deno was wrapping it in `import` statements and loading it as a
`data:text/javascript` URL via `load_main_es_module`, which enforces
strict mode. This broke libraries like fflate that use bare variable
assignments (e.g. `u8 = Uint8Array`) in their worker code.
The fix passes eval code via `hasSourceCode: true` so it goes through
V8's `execute_script` (sloppy mode) instead of the ES module loader.
Fixes #26739
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous commit unconditionally ran eval workers in sloppy mode via execute_script, which broke eval code using import/export syntax. Node.js auto-detects: code with import/export runs as ESM (strict), code without runs as CJS (sloppy). This commit replicates that behavior with a simple regex check for module syntax. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These tests now pass thanks to the sloppy mode and ESM/CJS auto-detection fixes for worker_threads eval code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
devsnek
reviewed
Mar 5, 2026
Per review feedback, worker eval code should always run as CJS (sloppy mode) — remove the ESM auto-detection path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Since eval workers now unconditionally run as CJS, all tests using `import` in eval worker strings need to use `require()` instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused `encodeURIComponent` import (lint failure after
removing the ESM eval path).
- Use `require("node:assert")` instead of `require("${common}")`
in napi init_test worker since `import.meta.resolve()` returns
a file:// URL that `require()` cannot resolve.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kajukitli
approved these changes
Mar 7, 2026
Contributor
kajukitli
left a comment
There was a problem hiding this comment.
lgtm — good fix for the strict mode vs sloppy mode mismatch
the approach is correct:
- using
hasSourceCode: truewithexecute_scriptruns the code in sloppy mode (V8 script evaluation) - the previous approach with
data:text/javascriptforced ESM (strict mode) - the
vardeclarations for CJS globals are correct sinceconst/letwould be block-scoped
test coverage:
- new
eval_sloppy_mode.mjstest with bare assignment (myGlobal = 42) validates sloppy mode - existing tests updated to use
require()instead ofimportwhich is more accurate to CJS
one thing to note: the PR description mentions fflate may have additional issues with transferred data — might be worth a follow-up issue if that's still broken
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
node:worker_threadswith{ eval: true }was wrapping code inimportstatements and loading it as adata:text/javascriptURL viaload_main_es_module, which enforces strict mode. Node.js runs eval workers as CommonJS (sloppy mode).u8 = Uint8Array) in their serialized worker code fail withReferenceError: u8 is not defined.hasSourceCode: trueso it goes through V8'sexecute_script(sloppy mode) instead of the ES module loader. CJS globals (require) are already provided by the Node worker bootstrap; we just add__filename,__dirname,module, andexportsviavardeclarations.Fixes #26739
Fixes #21695