Skip to content

fix(ext/node): run worker_threads eval code in sloppy mode#32428

Merged
bartlomieju merged 8 commits intomainfrom
fix/worker-threads-eval-sloppy-mode
Mar 10, 2026
Merged

fix(ext/node): run worker_threads eval code in sloppy mode#32428
bartlomieju merged 8 commits intomainfrom
fix/worker-threads-eval-sloppy-mode

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

@bartlomieju bartlomieju commented Mar 3, 2026

Summary

  • Root cause: node:worker_threads with { eval: true } was wrapping code in import statements and loading it as a data:text/javascript URL via load_main_es_module, which enforces strict mode. Node.js runs eval workers as CommonJS (sloppy mode).
  • Impact: Libraries like fflate that use bare variable assignments (e.g. u8 = Uint8Array) in their serialized worker code fail with ReferenceError: u8 is not defined.
  • Fix: Pass eval code via hasSourceCode: true so it goes through V8's execute_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, and exports via var declarations.

Fixes #26739
Fixes #21695

bartlomieju and others added 3 commits March 3, 2026 13:04
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>
Comment thread ext/node/polyfills/worker_threads.ts Outdated
bartlomieju and others added 5 commits March 5, 2026 16:42
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>
@bartlomieju bartlomieju requested a review from devsnek March 6, 2026 09:05
Copy link
Copy Markdown
Contributor

@kajukitli kajukitli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm — good fix for the strict mode vs sloppy mode mismatch

the approach is correct:

  • using hasSourceCode: true with execute_script runs the code in sloppy mode (V8 script evaluation)
  • the previous approach with data:text/javascript forced ESM (strict mode)
  • the var declarations for CJS globals are correct since const/let would be block-scoped

test coverage:

  • new eval_sloppy_mode.mjs test with bare assignment (myGlobal = 42) validates sloppy mode
  • existing tests updated to use require() instead of import which 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

@bartlomieju bartlomieju enabled auto-merge (squash) March 9, 2026 17:05
Copy link
Copy Markdown
Contributor

@nayeemrmn nayeemrmn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartlomieju bartlomieju merged commit 88d6de0 into main Mar 10, 2026
114 checks passed
@bartlomieju bartlomieju deleted the fix/worker-threads-eval-sloppy-mode branch March 10, 2026 14:44
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.

Deno issue with fflate workers RangeError: Maximum call stack size exceeded when using z3-solver

4 participants