feat(worker): resolve a raw-string Worker specifier caller-relative at transpile time - #141
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — a transpile-time rewrite that resolves a raw-string new Worker("…") specifier caller-relative (like a top-level import), fixing node:worker_threads' process.cwd()-relative footgun.
- Add the
worker_rewriteoxc pass — rewrites arg[0] of anew Worker(<string>)only when the callee is the free/globalWorker(symbol_id().is_none()scope guard) and the arg is a string literal; recurses children before rewriting so the emitted node is never re-visited. - Override
CompilerInterface::transform— runs oxc's stock transform first (byte-parity), then the additive rewrite over the same allocator +Scoping; gated onsource_type.is_module()and a non-emptyfile_paththreaded in from the napitransform()entry. - Refactor
resolve_tsinto a borrowingresolve_ts_core— reused by the rewrite, preserving theSome(nub-owned) /None(Node-owned) resolver boundary. - Add the napi-free
nub-worker-resolvecrate — pure path helpers (is_bare_specifier,relativize_for_url,has_url_scheme) carrying the soundness moat (cross-root ⇒None⇒ never bake an absolute path), unit-tested on every platform including Windows drive/separator cases. - Hybrid emit — nub-owned →
new URL("<rel>", import.meta.url); bare package →import.meta.resolve("<bare>"); absolute /file:/data:/blob:/node:/URL/ computed args pass through byte-identical. - Tests + fixtures + docs — foreign-cwd e2e for relative / extensionless / tsconfig-
pathscases, aworker_threads-binding non-rewrite guard, compat-mode (--node/NODE_COMPAT=1) opt-out assertions, and aworkers.mdxupdate documenting the convenience form + the bare-package bundler caveat.
The design is sound and the test coverage is strong — the scope guard is exercised with a relative path (not a passthrough absolute), the soundness moat is unit-tested, and the foreign-cwd setup genuinely proves caller-relative resolution. I verified the one load-bearing third-party contract: the bare-package emit relies on import.meta.resolve being synchronous and unflagged, which Node made true in v18.19.0 — exactly nub's support floor — so there is no floor-tier regression.
ℹ️ The transform override re-implements oxc's default transform body
The new CompilerInterface::transform override calls Transformer::new(...).build_with_scoping(scoping, program) directly to preserve byte-parity with oxc's stock transform, then runs the additive rewrite. That mirrors what the trait's default transform does today — but it's a hand-copied duplication of upstream behavior, so a future oxc bump that changes the default transform's body (extra passes, different scoping handling) would silently not be tracked here.
This is awareness-only, not actionable: the change is correct against the pinned oxc =0.132.0, and the existing transpile tests (which exercise this same path on no-Worker files) should surface any divergence on a bump.
Technical details
# transform override duplicates oxc's default `transform` body
## Affected sites
- `crates/nub-native/src/transform.rs:557-587` — override of `CompilerInterface::transform`; the `Transformer::new(...).build_with_scoping(...)` line re-implements the trait default so the worker rewrite can run over the same allocator + scoping.
## Required outcome
- No code change required for this PR. When bumping oxc (the version is pinned `=0.132.0` and tracked in lockstep with the `oxc-transform` npm pin), re-confirm the default `CompilerInterface::transform` body still matches this override, since the two must stay in sync for byte-parity to hold.
## Suggested approach (optional)
- Consider a one-line comment pointing at the upstream default `transform` impl (path/version) so the next oxc-bump author knows to re-diff it. Optional — the existing comment already explains *why* the seam was chosen.Claude Opus | 𝕏
3d39207 to
e28f3d5
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the delta since the prior pullfrog review (3d39207 → e28f3d5, same parent 1889904), which hardens the URL-encoding of the emitted worker specifier and tightens the passthrough guard.
- Rewrote
encode_url_pathas an RFC-3986 allow-list — unreserved ASCII (A-Za-z0-9.-_~) and/pass through verbatim; everything else, including every non-ASCII UTF-8 byte, is percent-encoded per byte. This fixes a real bug in the prior deny-list version, which cast non-ASCII bytes viab as charand emitted latin-1 mojibake (é→é);café.tsnow correctly becomes./caf%C3%A9.tsand round-trips throughnew URL→fileURLToPath. - Added a degenerate-specifier passthrough guard —
is_passthroughnow short-circuits empty / whitespace-only / bare./..specifiers before resolution, so the rewrite never emits a nonsensicalimport.meta.resolve(""). - Moved the compat-mode test to a plain
.mjsfixture —compat-main.mjsreplaces the.tsentry because--nodedisables the transpiler, so a TS annotation would throwSyntaxErrorbefore reachingnew Workerand mask theWorker is not definedsignal. - Added unicode / URL-hazard coverage —
percent_encodes_non_ascii_utf8_bytes(Latin + CJK),percent_encodes_url_hazard_ascii,output_is_never_absolute, pluscafé.ts/unicode-main.tse2e fixtures run from a foreign cwd.
The encoding fix is correct and well-covered: the per-byte UTF-8 percent-encoding is the right approach for a new URL first-arg literal, and the round-trip is exercised both as a unit test and end-to-end. The degenerate guard lands in is_passthrough (ahead of resolve_ts_core), which is the right seam. The prior review's lone informational note — the transform override duplicating oxc's default transform body — is unchanged and remains awareness-only.
Claude Opus | 𝕏
…uniformly across module languages
A raw string in `new Worker("./worker.ts")` resolves with nub's full module
resolution against the calling module — like a top-level import — instead of
node:worker_threads' cwd-relative behavior. A net-new oxc VisitMut pass (run from
the transform seam, which holds both the allocator and post-transform scoping)
rewrites the literal:
- nub-owned (relative / extensionless / tsconfig `paths`) → re-relativized
`new Worker(new URL("<rel>", import.meta.url))` (the bundler-analyzable form);
- bare package (resolver returns None) → `new Worker(import.meta.resolve("<bare>"))`,
so Node owns node_modules / exports.
Uniform across module LANGUAGES: a `.ts`/`.tsx`/`.mts`/`.cts`/`.jsx` caller is
already transpiled, and a plain `.js`/`.mjs`/`.cjs` ES module — which nub does NOT
transpile by default (the #152 byte-parity no-op invariant) — is routed through the
transform by an added trigger in `maybeTranspilePlainJs`: a precise,
semantic-scoped `has_global_worker_string_call` detection (a bound/shadowed Worker
does NOT trigger, so a no-Worker / no-lowerable file stays byte-identical). The
single `resolve_worker_target` resolves a relative specifier for any caller
extension, so a `.js` caller behaves identically to a `.ts` caller. CJS modules are
left as-is (import.meta is ESM-only) — the one fundamental, language-independent
distinction.
Scope-guarded to the FREE/global `Worker` (symbol_id().is_none()): a
node:worker_threads import or a shadowed local is left untouched. node_modules is
never transpiled/rewritten (the byte-parity boundary). Passthrough — absolute /
file: / data: / blob: / new URL / a computed arg — left byte-identical. Never bakes
an absolute path (cross-root → untouched); non-ASCII paths percent-encode their
UTF-8 bytes and round-trip.
Claude-Session: https://claude.ai/code/session_012YwF7yWjrXcj3bVJ1TziHK
e28f3d5 to
ffbdfb2
Compare
There was a problem hiding this comment.
ℹ️ No critical issues — one minor resolution-gap suggestion inline.
Reviewed changes — the delta since the prior pullfrog review (e28f3d5 → ffbdfb2), which rebased the PR onto a newer main (70b0a63) and extended the worker-string rewrite from TS-only to raw plain-JS (.js/.mjs/.cjs) ES-module callers, uniformly with .ts.
- Add a worker trigger to
maybeTranspilePlainJs— a plain-JS file with no lowerable syntax now also routes through the transform when it carries an ESMnew Worker(<string-literal>), so.js/.mjscallers resolve caller-relative like.ts. ESM-gated (info.hasGlobalWorkerStringCall && isEsm), so a CJS worker file stays byte-identical. - Add
has_global_worker_string_call+transformable_syntaxtodetect.rs— a two-pass scheme: a cheap pre-semanticSyntaxFinderover-approximation, then a preciseWorkerScopeCheckersemantic pass run only when a candidate exists, confirming free/global viasymbol_id().is_none()(the same guard the rewrite uses). - Add
resolve_worker_targettoresolve.rs— wrapsresolve_ts_coreand, onNonefor a relative spec, adds a parent-ext-agnostic relative probe so a raw-JS caller's./worker.jsresolves. - Make
.mjs/.cjsexplicit inmoduleFormatFor—.mjs→module,.cjs→commonjs, so the plain-JS gate gets the right format without a redundant detect. - Tests + fixtures + docs — foreign-cwd e2e for
.mjs,.js-as-ESM, CJS not-rewritten, node_modules exclusion, and a no-worker byte-parity no-op;workers.mdxdocuments the relative-string + bare-package forms and the uniform-across-languages behavior.
The extension is sound. The two-pass detection is self-consistent with the rewrite's own scope guard (both treat an unresolved reference_id as not-eligible), the JS ESM gate aligns with transform.rs's source_type.is_module() gate so a file is never transpiled-then-loaded as a different format, the node_modules byte-parity boundary is enforced at all three call sites, and CJS callers stay byte-identical. Test coverage for the new plain-JS dimension is strong and the foreign-cwd setups genuinely prove caller-relative resolution.
ℹ️ Nitpicks
- The two
detectModuleInfofallbacks inruntime/transform-core.mjs(the no-addon return and the catch return) omithasGlobalWorkerStringCall, so the new consumer readsundefined. It's safe by falsiness — these are error/no-addon paths that skip transpilation anyway — and the lines sit on the rebased base, not this PR's diff, so no change is needed; flagging only becausetransformableSyntax: falsewas added there while this field was not, leaving the return shape inconsistent.
Claude Opus | 𝕏
| let target = path_join_resolve(&parent_dir, specifier); | ||
| // Empty parent_ext → default probe order (.ts/.tsx/.js/.jsx/.json); the | ||
| // `.js`→`.ts` emit-swap still applies for an explicit `.js`. | ||
| if let Some(resolved) = try_resolve_file(&target, "", true) { |
There was a problem hiding this comment.
The fallback probe discards the caller's extension — try_resolve_file(&target, "", true) passes an empty parent_ext, so probe_order("") is .ts/.tsx/.js/.jsx/.json and never probes .mjs/.cjs. An extensionless new Worker("./worker") from a .mjs/.cjs caller whose only sibling is ./worker.mjs (or .cjs) silently returns None and is left unrewritten, even though the explicit-extension form (./worker.mjs) resolves fine via the emit-swap above. This is a narrow inconsistency with the "uniform across module languages" claim — extensionless probing reaches .ts/.js siblings for a .mjs caller but not .mjs/.cjs ones.
Technical details
# resolve_worker_target extensionless probe ignores the caller extension
## Affected sites
- `crates/nub-native/src/resolve.rs:103` — `try_resolve_file(&target, "", true)` hardcodes an empty `parent_ext`.
- `crates/nub-native/src/resolve.rs:190-197` — `probe_order("")` falls to the `_` arm (`.ts/.tsx/.js/.jsx/.json`); the `.mts`/`.cts` arms add `.mjs`/`.cjs` but are unreachable from the empty string.
## Required outcome
- An extensionless relative worker specifier from a `.mjs`/`.cjs`/`.mts`/`.cts` caller should probe the caller-appropriate extensions (so `./worker` reaches a `./worker.mjs` sibling), matching the explicit-extension behavior and the uniform-across-languages claim.
## Suggested approach (optional)
- Compute `extname(parent_path)` once in `resolve_worker_target` and pass it through to `try_resolve_file` instead of `""`, so the probe order reflects the caller. Add a fixture exercising an extensionless `new Worker("./worker")` whose only sibling is `.mjs` to lock it in (current `.mjs` fixtures use an explicit `.js` target, so this gap is uncovered).
## Open questions for the human (optional)
- Is extensionless-to-`.mjs`/`.cjs` worker resolution in scope, or is it acceptable to require an explicit extension for those callers (matching plain Node ESM, which requires extensions on relative imports)? If the latter, this is intentional and can be left as-is.Reverts #141. A raw relative `new Worker("./w")` resolves cwd-relative again, matching node:worker_threads and Bun, with no transpile rewrite. #141 made a raw string in new Worker("./w") resolve caller-relative at transpile time. That rewrite only fires under nub's own runtime, so a third-party bundler operating on the user's source never sees it and the same code resolves differently once built. The explicit forms already cover the need without an implicit rewrite: import.meta.resolve("./w") (caller-relative at runtime) and new URL("./w", import.meta.url) (also the idiom bundlers trace to chunk the worker). Surgical revert of the ce80a62 squash. Removed: the nub-worker-resolve crate, worker_rewrite.rs, the worker-string trigger in detect.rs / transform-core.mjs, and the #141 fixtures/tests. Kept intact: #152 (plain-JS transpile) and #99 (the web-Worker polyfill, shipped v0.2.1) -- the explicit new URL / import.meta.resolve forms still resolve through its file:// branch. Docs updated accordingly.

What
A raw string in
new Worker("./worker.ts")resolves with nub's full module resolution against the calling module — like a top-level import — instead ofnode:worker_threads' cwd-relative behavior. Running a script from a different directory than the one that constructs the worker no longer breaks.This is uniform across module languages: a
.ts/.tsx/.mts/.cts/.jsxcaller and a plain.js/.mjs/.cjsES module all behave the same.How
A net-new oxc
VisitMutpass (run from the transform seam, which holds both the allocator and the post-transform scoping) rewrites the first argument of anew Worker(<string-literal>)when the callee is the free/globalWorker:paths) → re-relativizednew Worker(new URL("<rel>", import.meta.url))— the canonical, bundler-analyzable form;new Worker(import.meta.resolve("<bare>"))— Node's own resolver owns node_modules /exports.Uniform across
.tsand plain JS.ts/.jsx/… callers are already transpiled, so the pass runs there directly. A plain.js/.mjs/.cjsES module is not transpiled by default (the #152 byte-parity no-op invariant), so it's routed through the transform by an added trigger inmaybeTranspilePlainJs: a precise, semantic-scopedhas_global_worker_string_calldetection. A singleresolve_worker_targetresolves a relative specifier for any caller extension, so a.jscaller resolves identically to a.tscaller.The byte-parity no-op invariant is preserved:
Worker, a non-string-literal arg, or no worker at all → does not trigger, so the file stays byte-identical;import.meta.url, which is ES-module-only — and is not transpiled, so a CJS file with a worker string stays byte-identical (no needless codegen);Guards
symbol_id().is_none()): only the globalWorkeris rewritten — animport { Worker } from "node:worker_threads"or a shadowed local is left untouched.new URL→fileURLToPath.file:/data:/blob:URL strings, anew URL(...)arg, aURLinstance, any computed/dynamic arg.--node/NODE_COMPAT=1disable augmentation entirely, so the bare string behaves like plain Node.The portability-critical path helpers live in a napi-free
nub-worker-resolvecrate, unit-testable on every platform (Windows separators/drives the Linux e2e can't reach).Tests
Worker-string cases for
.ts/.tsx,.mjs,.js-as-ESM, and.cjscallers; the scope guard; tsconfig-paths; non-ASCII round-trip; the byte-parity no-op invariant (a no-worker.mjsand a CJS worker file both stay native); node_modules exclusion. Green on the fast (22.15) and compat (18.19) tiers + Node 26.Caveat
A bare-package worker (
new Worker("pkg/worker")) resolves viaimport.meta.resolve, which is not statically analyzable by bundlers — the same way a dynamicimport.meta.resolveisn't. This is the price of node_modules/exportsparity; the common relative /new URLcases stay fully static.This is a runtime-behavior change to a shipped API (the bare-string Worker resolved cwd-relative before; it's now caller-relative). Flagging it for the maintainer.
Refs the
worker-relative-specifier-resolutiondesign thread.https://claude.ai/code/session_012YwF7yWjrXcj3bVJ1TziHK