Repository navigation
Conversation
…ssions module.register() has been runtime-deprecated as DEP0205 since Node.js v25.9.0, emitting a deprecation warning on every invocation. Switch to the synchronous, in-thread module.registerHooks() (Node.js >= 23.5.0 / 22.15.0) when available, falling back to module.register() on older runtimes. The synchronous loader routes CommonJS require() and named-export detection through the customization hooks, which surfaced several interop differences that the old worker-thread loader hid. Fix them so behaviour is unchanged: - Defer modules oxc-node does not transform (plain .js/.cjs/.json, native addons, …) and anything Node classifies as commonjs back to Node.js, so its built-in CommonJS named-export detection (incl. transitive __export(require()) re-exports) and pirates-based transpilation/source maps keep working. - Complete missing TypeScript/JSX extensions for CommonJS require() specifiers, which the ESM-style resolve hook's nextResolve does not resolve via Module._extensions where pirates installs its handler. - Make ResolveContext/LoadContext conditions and importAttributes optional, as the CommonJS require() path does not always provide them. - Move the loader-thread keep-alive MessageChannel out of the shared hook module so it only runs on the dedicated register() loader thread, instead of pinning unrelated worker threads (e.g. test runners) and preventing process exit. Verified on Node.js 24 and 26 with the integrate-module, integrate-module-bundler and integrate-ava suites all green and no DEP0205 warning.
Replace the filesystem-probing (existsSync) extension completion for CommonJS require() of TypeScript files with a dedicated native resolver entry, resolveCjsSpecifier(). It reuses oxc-node's resolver so tsconfig `paths`, package `exports`, conditions and symlinks are all honoured (the previous best-effort probing only handled relative/absolute specifiers), and removes the synchronous filesystem stat loop from the hot resolve path.
- Replace the per-resolve/per-load `new URL(url).pathname` transformability check
with a substring regex on the raw string (~9.6x faster on the hot path; extension
characters are never percent-encoded so this is safe), and check the cheap
`format === 'commonjs'` branch before it in the load hook.
- Load index.js via createRequire instead of a named ESM import. The native binding's
exports are only statically detectable once oxc-node's hooks are active, so a named
`import { … } from './index.js'` could fail to find exports under some bootstrap
entrypoints (e.g. --eval). This also avoids ESM named-export detection overhead.
A specifier that already carries a transformable extension (e.g. `./foo.ts`) is always oxc-node's to resolve, so the speculative Node `nextResolve` — whose result would only be discarded — can be skipped, resolving relative TypeScript imports once instead of twice. ~18% faster on a TypeScript-heavy module graph (200 modules: 36.5ms -> 29.9ms).
For modules oxc-node has fully resolved outside node_modules (URL + concrete format already known), build the resolve hook output directly rather than calling nextResolve with the resolved URL. The latter made Node redundantly re-read package.json and stat the file for a path oxc-node had already resolved. node_modules modules still defer to Node so CommonJS named-export detection is unaffected. ~20% faster on a TypeScript-heavy module graph (500 modules: 70.5ms -> 56.2ms).
The synchronous-loader fast paths in hooks.mjs rely on a synchronous nextResolve (only true under module.registerHooks()). Under the module.register() fallback, hooks run on a worker thread where nextResolve is asynchronous, so the try/catch guarding the speculative resolve cannot catch its rejection and tsconfig paths aliases threw ERR_MODULE_NOT_FOUND. The async worker loader also does not have the CommonJS interop limitations hooks.mjs works around, so the register() entry (esm.mjs) now uses the plain resolver directly, while registerHooks keeps using the optimized hooks.mjs.
|
@Boshen Please review this PR. Thanks. |
|
Hi @Brooooooklyn ? @Boshen ? |
|
@Boshen Is this project dead? |
|
Thanks for this — the goal is right and I want it to land. But I can't merge it as it stands. I built both branches and ran the same fixtures against The suite is green on all three packages ( Blocking1.
|
… findings
Reworks the branch along the lines of the review: the three performance changes
are gone, the migration is gated on a version rather than a feature check, and
every behavioural difference from `main` that the review found is fixed and
covered by a test.
The loader
----------
`hooks.mjs` is now the migration and nothing else. `resolve` and `load` hand the
whole CommonJS `require()` path back to Node.js, which is where the asynchronous
`module.register()` loader left it too — its hooks cannot serve a synchronous
`require()`, so `require()` never reached them. Node.js' own CommonJS resolution
honours `Module._extensions`, where `register.mjs` installs `pirates`, so
`require("./foo")` finds `foo.ts` and still prefers `foo.json` over `foo.ts`.
The `resolveCjsSpecifier()` binding the previous revision added for this is
therefore unnecessary and has been removed: the napi surface is unchanged.
The `load` hook additionally defers any module `pirates` will transpile, because
Node.js compiles CommonJS through `Module._extensions` *after* calling the hook;
transforming it in both places registered two source maps for one file and made
stack traces point at generated positions.
`esm.mjs` is byte-identical to `main` again, so the `module.register()` fallback
is unchanged.
The version gate
----------------
`supportsRegisterHooks()` enables the synchronous hooks on Node.js >= 24.18.0 and
>= 26.2.0 only. Before those releases a `require()` made from a CommonJS module
that Node.js itself loaded through the ESM CommonJS translator was routed through
the *ESM* resolver: the `resolve` hook was handed the resolved URL with the
`import` condition set, which a loader cannot tell apart from a real `import`, so
it hands the CommonJS loader an ES module. Feature detecting `registerHooks`
(v22.15.0 / v23.5.0) or the array-shaped conditions (v22.19.0 / v24.5.0) is not
enough. Node.js 22, 23 and 25 keep using `module.register()`; only v26.0.x and
v26.1.x both need the fallback and warn about it (DEP0205 lands in v26.0.0).
Behaviour restored
------------------
* `LoadContext.format` is optional — the `require()` path does not always report
one, which made `require()` of `.tsx` / `.jsx` / `.es` / `.es6` fail with
"Missing field `format`".
* JSON imports without an import attribute work again: the resolve hook no longer
short-circuits `.json` away from the synthesised named exports.
* `create_resolve` calls `next_resolve` with the resolved URL again instead of
returning its own output, so tsconfig `paths` beats `node_modules`, downstream
hooks keep seeing every module, and Node.js re-validates the URL.
* Plain `.js` / `.mjs` / `.cjs` files are transformed again, and
`OXC_TRANSFORM_ALL` still reaches dependencies.
* `register.mjs` has no top-level `await`, so `node -r …/register.mjs` works.
Fixed at the source
-------------------
* `oxc_resolved_path_to_url` percent-encodes the path (WHATWG path percent-encode
set) and `create_resolve` percent-decodes `file:` specifiers and parent URLs, so
a project path containing a space or a non-ASCII character resolves and keeps a
single module identity. Relative imports from inside such a directory now work,
which they did not before.
* Resolvers are cached per export-condition set instead of first-writer-wins,
cloned from one base resolver so they share its filesystem cache and tsconfig.
A CommonJS transform running first no longer freezes the resolver on an empty
condition set.
* File extensions are matched against a URL's `pathname`, so a `?query` or
`#fragment` cannot be mistaken for an extension.
* The napi bindings are regenerated from a wasm build, which is the only target
that refreshes `oxc-node.wasi*.{cjs,js,d.cts}`.
Tests and CI
------------
`packages/integrate-vitest/__tests__/loader.spec.ts` adds 26 specs covering each
of the above in a child process, plus a table for the version gate. They pass on
both loader implementations.
CI gains a `test-register-fallback` job on Node.js 22.18.0 — the oldest release
the toolchain supports and one that takes the `module.register()` path, which no
other job exercised — and the wasm build job now fails if the committed napi
bindings drift.
Known differences from `main`
-----------------------------
`module.registerHooks()` has a single hook chain and runs the most recently
registered hook first, so a hook registered *before* `@oxc-node/core/register`
now runs after it and is handed the resolved URL instead of the original
specifier. It still observes every module; registering it after oxc-node restores
the old view. This is inherent to the synchronous API and is documented in the
README and pinned by a test.
Fixes the root cause behind the CommonJS interop problems in this branch instead
of gating around them.
Oxc does not lower ES modules to CommonJS — `Module::CommonJS` and
`Module::Preserve` both leave `import`/`export` in place — and the helper loader
*adds* `import` declarations to files that need a runtime helper. The `load`
binding nevertheless echoed Node.js' own classification back, so oxc-node
routinely handed Node.js ES module source labelled `commonjs`. That only worked
where Node.js happened to re-detect the module syntax while compiling
(`loadESMFromCJS`); everywhere else it failed with
`SyntaxError: Unexpected token 'export'`, or silently produced a CommonJS facade
with no named exports.
`transform_output` now reports `module` when the code it generated is an ES
module, using the parser's `has_module_syntax` for the input plus the module
declarations present after the transform. `hooks.mjs` uses that verdict to decide
ownership of CommonJS-classified modules: when the output really is CommonJS,
Node.js compiles the file through `Module._extensions` and ignores the source the
hook returned, so the hook hands back the untouched original instead of
registering a second, conflicting source map for it.
Consequences:
* The version gate drops from v24.18.0 / v26.2.0 to v22.22.3 / v24.8.0 — the
first releases where Node.js can execute an ES module produced for a
`require()` on this path. Every Node.js version that runtime-deprecates
`module.register()` (DEP0205, v26.0.0 and later) now uses the synchronous
hooks, so the warning is gone entirely, and both current LTS lines get the
in-thread loader.
* `import { … } from "./x.es6"`, and from a `.ts` file inside a
`"type": "commonjs"` package, keep their named exports. Both silently lost them
before, on `main` as well.
* `node -r @oxc-node/core/register` works on every supported version instead of
only the ones whose asynchronous loader implements `resolveSync()`.
Verified against `main` on 21 Node.js releases from 22.15.0 to 26.8.1 with a
24-scenario matrix: no behavioural difference other than the documented hook chain
ordering, plus the three fixes above. The full test suite passes on 15 of those
releases with `OXC_TRANSFORM_ALL` both set and unset.
Also stops `stdin-tty.spec.ts` from failing on any Node.js version that emits a
process deprecation warning, and points the `test-register-fallback` CI job at
22.18.0 and 24.7.0 — the last releases below the new gate.
…olver leak
Follow-up review found one regression and two latent defects; the other reported
issues were verified against `main` and are not real.
Fixed:
* `transform_output` derived the file extension with `Path::extension()` on the whole
URL, so `data.json?v=1` reported `json?v=1`, skipped the JSON branch and handed the
JSON text to the JavaScript parser. `main` got away with it because its asynchronous
loader returns `source: null` for CommonJS and never reaches the transform, while the
synchronous loader returns the real source — so this was a regression in this branch.
Extensions are now read from the URL's path, in `transform_output` and in the resolve
hook's `.json` short circuit.
* `oxc_resolved_path_to_url` did not escape `%`, the escape character itself, so a path
containing one produced a URL Node.js rejects with `URI malformed`. Broken on `main`
too; now that the function percent-encodes, encoding `%` completes it.
* The per-condition resolver cache leaked a `Resolver` through `Box::leak` for every
distinct condition set, which a custom hook can keep inventing. They are reference
counted now.
Verified as not real:
* CommonJS globals in a CommonJS-scoped `.ts` file. `__dirname` is already `undefined` on
`main` for any such file that has module syntax, because oxc emits `export {}` for
erased type exports and Node.js then loads it as an ES module either way. Behaviour is
identical; the `import` case goes from a hard failure to working.
* Conditional `exports` picking the `import` branch for a `require()` below the version
gate. Checked on 22.18.0 through 26.8.1, from a translator-loaded CommonJS entry, a
nested `require()` and `createRequire()`: every version resolves the `require` branch,
because Node.js resolves the specifier before the hook sees it.
Tests and CI:
* `spawnSync` now has a timeout and asserts the child was not killed — it blocks the event
loop, so Vitest's timeout could not interrupt a child kept alive by the loader.
* The two "was it transformed" specs parsed whole stdout/stderr and only asserted the two
runs differed, which `OXC_LOG`/`DEBUG` output and temporary paths could satisfy on their
own. They assert the reported value now.
* New specs for JSON with `?query` and `#fragment`, and for a path containing `%`.
* The bindings drift check no longer misses a generated file that was never committed.
* `hooks.mjs` is internal, so its declaration file is no longer published.
* README spells out that Node.js 23 is excluded rather than implying "22.22.3 and later".
The resolve hook reports `module` for a JSON module imported without an import attribute, so that the load hook can synthesise one named export per key. For a JSON array or scalar there are no keys, and the load hook reported `commonjs` with a `module.exports = …` body instead. Under the asynchronous `module.register()` loader Node.js executes that source directly, so it worked. The synchronous loader routes a `commonjs` JSON URL to `Module._extensions[".json"]`, which `JSON.parse`s whatever the hook returned and fails with `Unexpected token 'm', \"module.exp\"... is not valid JSON`. Emit `export default <json>` with format `module` instead, matching both the declared format and the object branch right above it. Adds specs for a JSON array, number and string.
cjnoname
changed the title
|
Thanks — this was a genuinely useful review. I reworked the branch and the description is rewritten; summary of what changed, and two places where the diagnosis turned out to be different from what either of us thought. The three perf changes are gone.
So the binding was unnecessary. What actually breaks The The root cause behind all the CommonJS interop pain. Oxc does not lower ES modules to CommonJS — That is also what lets the gate sit at v22.22.3 / v24.8.0 instead of v24.18.0: every version that emits DEP0205 is above it, so the warning is gone completely, and both current LTS lines get the in-thread loader. On the rest of the list: Finding 9 I could not fully fix, and I would rather say so than paper over it. Verification. A 24-scenario matrix against Every finding has a spec in Two unrelated bugs I hit while writing those tests are split out rather than bundled here:
Both are independent of this PR and of each other. Windows UNC file URLs are still unsupported — real, but identical to Happy to split this further if you would rather review the migration and the format fix separately. |
Finding 9 of the review, which the previous revision documented as inherent to `module.registerHooks()`. It is not. The native resolver resolves the specifier and then calls `nextResolve` with the resolved URL, to have Node.js validate it and fill in the resolution metadata. Under `module.register()` that was invisible: oxc-node's hooks ran on a separate loader thread, after every in-thread hook, so those hooks always saw the original specifier. With a single chain oxc-node runs first, and passing the resolved URL down hid the specifier from module mocking and policy hooks. `resolve` now wraps `nextResolve` so the rest of the chain is asked about the specifier as written, and only falls back to the resolved URL when Node.js cannot resolve it — a tsconfig `paths` alias, an extensionless TypeScript file. A hook registered before oxc-node observes `./dep.ts` again, exactly as under `module.register()`, and the 24-scenario matrix across Node.js 22.18.0 to 26.8.1 now shows no case where this branch reports something different from `main`. What is left is narrower and now has a test of its own rather than a paragraph: oxc-node resolves first, so its URL wins over a *redirect* from a hook that runs after it. Registering the hook after oxc-node makes the redirect win, on both loaders.
|
Follow-up on finding 9: you were right that it mattered, and I was wrong to call it inherent. It is fixed now. The native resolver resolves the specifier and then calls
function withOriginalSpecifier(specifier, nextResolve) {
return (resolved, context) => {
if (resolved !== specifier) {
let output;
try {
output = nextResolve(specifier, context);
} catch {
return nextResolve(resolved, context);
}
if (output !== null && typeof output === "object" && !("then" in output)) {
return { ...output, url: resolved };
}
}
return nextResolve(resolved, context);
};
}A hook registered before oxc-node observes What remains is narrower and has its own test rather than a paragraph: oxc-node resolves first, so its URL wins over a redirect from a hook that runs after it. Registering the hook after oxc-node makes the redirect win, on both loaders. The test asserts both orders and both loaders. Also filed #744 for the Windows UNC file URL handling — it was only a sentence in the description before, which was the wrong place for it. Behaviour is the same on |
`engines` said `>= 20.6.0`, the release that added `module.register()`. That was an overclaim: oxc does not lower ES modules to CommonJS, so `require()` of a transpiled TypeScript file needs `require(esm)`, which only became available by default in the 20 line in **v20.19.0**. Below that it fails with `SyntaxError: Unexpected token 'export'` — on `main` as well, so this is a correction to the claim rather than a behaviour change. Bisected across 20.6.0, 20.10.0, 20.16.0, 20.17.0, 20.18.0, 20.18.3 and 20.19.0. The review asked for a CI job on the lowest supported version, and until now the lowest was 22.18.0, so the floor was not covered at all. pnpm itself requires Node.js >= 22.13 and cannot even start on 20, so `test-engines-floor` installs the suite under Node.js 22 and then runs `integrate-module` (21 specs) under 20.19.0 directly, after asserting that this version really does take the `module.register()` fallback.
|
One more correction, this time to something I introduced rather than to the original branch. I had added
The description now carries a table mapping each item of your unblock list to where it landed. Current state:
Two things I did not do: the follow-up PR for the three performance changes — they are just removed here, and can come back with the benchmarks and semantics tests you asked for — and Windows UNC file URLs, which are #744. Findings I got wrong along the way and corrected: the |
…lass-field polarity The spec asserted the transformed dependency reports `setterCalled: true`, which holds under the current `useDefineForClassFields` mapping and flips once oxc-project#742 corrects it. It now runs the dependency under both values of the option and asserts they disagree when it was transformed and agree when it was not, which is true under either mapping.
|
Cross-PR note, found by merging all three of these together and running the suites rather than assuming they compose. They do not conflict textually — git merges all three cleanly — but #742 changes the class-field semantics that fixtures in the other two relied on, so whichever landed second would have had red CI:
Both fixtures are now independent of it:
With those in place, all three merged together: 73/73 on Node.js 22.22.3, 24.8.0, 24.20.0, 26.2.0 and 26.8.1, 71 passed + 2 skipped on 22.18.0 and 24.7.0 (the two specs that need the synchronous hooks), and the No product code changed for this — only the two fixtures. |
|
Merge note for #633 and #745, found by merging all four open PRs together and building rather than trusting the merge result.
if !is_json
&& env::var("OXC_TRANSFORM_ALL")
.map(|value| value.is_empty() || value == "0" || value == "false")
.unwrap_or(true)
if env::var("OXC_TRANSFORM_ALL") // <- second copy of the condition
.map(|value| value.is_empty() || value == "0" || value == "false")
.unwrap_or(true)
&& url.contains("/node_modules/")
{Three things need doing by hand, all mechanical:
With those resolved, all four merged together: 87/87 on Node.js 22.22.3, 24.8.0, 24.20.0, 26.2.0 and 26.8.1, 85 passed + 2 skipped on 22.18.0 and 24.7.0 (the specs that need the synchronous hooks), Point 1 is the one worth watching: it is the only one the compiler does not catch. If #633 lands first and #745 is merged without noticing it, JSON goes back to being short circuited before the resolver and #726 silently regresses — with #745's own specs still passing, because the fallback path they exercise still reports the right format. I would rather flag it than let it slip through. |
Closes #726. ## The bug `create_resolve` short circuited on any specifier ending in `.json` **before its own resolver ran**, handing the unresolved specifier to Node.js. Node.js knows nothing about tsconfig `paths`, so an aliased JSON import failed: ```jsonc // tsconfig.json { "compilerOptions": { "baseUrl": ".", "paths": { "@data/*": ["./src/*"] } } } ``` ```ts import data from "@data/config.json"; // ERR_MODULE_NOT_FOUND import data from "@data/config.json" with { type: "json" }; // ERR_MODULE_NOT_FOUND ``` A `.ts` file behind the same alias resolves fine, so this is specific to JSON. Both forms failed — the attribute form because the separate "has import attributes" bail out also returns Node.js' resolution of the original specifier. ## The fix JSON is resolved like every other specifier, and the format is decided from the resolved path afterwards: - an import attribute was written → `json`, which is what Node.js wants - otherwise → `module`, so the `load` hook can synthesise a default export plus one named export per key When oxc-node's resolver cannot resolve the specifier the old short circuit still applies, so nothing changes for anything it already handled. Two things had to follow from letting JSON reach the resolver: **The extension has to be read from the URL's path.** `Path::extension()` on the whole URL reports `json?v=1` for `./data.json?v=1`. That was latent before — the specifier check kept those away from the resolver — and became observable immediately: the JSON branch was skipped and the JSON went to the JavaScript parser. **Turning JSON into a module is not a code transform**, so it no longer sits behind the `OXC_TRANSFORM_ALL` check for `node_modules`. That check is about transpiling dependency *source*; skipping this handed Node.js raw JSON to execute as an ES module. This also fixes a second broken case: ```ts // node_modules/pkg/package.json: { "exports": { "./config": "./cfg/real.json" } } import data from "pkg/config"; // before: ERR_IMPORT_ATTRIBUTE_MISSING after: works import { v } from "pkg/config"; // before: ERR_IMPORT_ATTRIBUTE_MISSING after: works import data from "pkg/config" with { type: "json" }; // worked before, still works ``` ## Verification `packages/integrate-vitest/__tests__/json-modules.spec.ts` covers the alias with and without an import attribute, named and dynamic imports through the alias, an `exports` subpath in a dependency, and the relative, `?query`, `#fragment`, array, number and string cases that already worked — the last group compared against `main` to confirm they are unchanged. `integrate-module` (21/21), `integrate-module-bundler` (22/22) and `integrate-vitest` (23/23) pass on Node.js 22.23.2 and 24.20.0 with `OXC_TRANSFORM_ALL` both set and unset, and `integrate-module` passes on 20.19.0. `cargo clippy --all-targets --all-features -- -D warnings` and `pnpm lint` are clean. No napi surface change. ## Note #633 contains the same `url_path` helper, for the same reason. The two are logically independent; whichever lands second drops the duplicate. --------- Co-authored-by: LongYinan <lynweklm@gmail.com> Co-authored-by: cjnoname <cjnoname@users.noreply.github.com>
2 participants
Summary
module.register()is runtime deprecated as DEP0205 from Node.js v26.0.0 and emits a warning on every invocation. This moves the loader to the synchronous, in-threadmodule.registerHooks()wherever Node.js can actually support it, and keepsmodule.register()as an unchanged fallback below that.Thanks for the detailed review — it was right about everything that mattered, and the branch has been reworked accordingly. The three performance changes are gone, the gate is a version check, and every behavioural difference from
mainis either fixed or documented and pinned by a test. Two of the findings turned out to have a different root cause than the original description claimed; details below.What the review asked for, and what happened
require()of TypeScript broken on older versions; gate must be a version checkSafeSet. See below.require()of.tsx/.jsx/.es/.es6→Missing field 'format'LoadContext.formatis optional.oxc_resolved_path_to_urlpercent-encodes,create_resolvepercent-decodes.next_resolveis called again as well.pathsloses tonode_modulescreate_resolveis called first again, so oxc-node's resolver still wins.require()resolution order invertedrequire()picks theimportbranch of anexportsmap.js/.mjs/.cjsno longer transformed;OXC_TRANSFORM_ALLa no-opresolveasks the chain about the specifier as it was written.TRANSFORM_EXTENSIONunanchored, query/fragment countspathname.awaitbreaksnode -rregister.mjshas no top-levelawait.esm.mjscovered by nothingtest-register-fallbackjob runs the versions below the gate.resolveCjsSpecifieris gone, and finding 1 had a different causeThe previous description claimed the resolve hook's
nextResolvedoes not consultModule._extensions, wherepiratesinstalls its handler. That is not true. A probe that registers a custom extension afterregisterHooks()shows Node.js honouring it on therequire()path regardless of registration order:So the whole
resolveCjsSpecifier()binding was unnecessary.resolveandloadnow hand the entire CommonJSrequire()path back to Node.js, which is where the asynchronous loader left it too — its hooks cannot serve a synchronousrequire(), sorequire()never reached them. That deletes the napi export (the surface is now identical tomain), fixes finding 6 by construction, and removes most ofhooks.mjs.The real reason
require()broke is different, and it is what the version gate is for. Until v24.18.0 / v26.2.0, arequire()made from a CommonJS module that Node.js itself loaded through the ESM CommonJS translator was routed through the ESM resolver: the hook received the already resolved URL together with theimportcondition set. A loader cannot tell such arequire()apart from a realimport, so it hands the CommonJS loader an ES module.The
SafeSetconditions the review found are real (getCjsConditionsArray()landed in v22.19.0 / v24.5.0) but sit below that floor, so they are covered by the same gate.The actual root cause of the CommonJS interop trouble
Oxc does not lower ES modules to CommonJS —
Module::CommonJSandModule::Preserveboth leaveimport/exportin place — and the helper loader addsimportdeclarations to files needing a runtime helper. Theloadbinding nevertheless echoed Node.js' classification straight back, so oxc-node routinely handed Node.js ES module source labelledcommonjs. That only worked where Node.js happened to re-detect the module syntax while compiling (loadESMFromCJS); on the CommonJS paths of the synchronous loader it does not.transform_outputnow reportsmodulewhen the code it generated is an ES module, using the parser'shas_module_syntaxplus the module declarations present after the transform.hooks.mjsuses that verdict to decide ownership: when the output really is CommonJS, Node.js compiles the file throughModule._extensionsand ignores the source the hook returned, so the hook hands back the untouched original instead of registering a second, conflicting source map for it.Fixing the mislabel is what lets the gate sit at v22.22.3 / v24.8.0 — the first releases where Node.js can execute an ES module produced for a
require()on this path. Every version that emits DEP0205 (v26.0.0+) is above it, so the warning is gone entirely, and both current LTS lines get the in-thread loader.The unblock list
typeof registerHookssupportsRegisterHooks()inpackages/core/hooks.mjsengines>= 20.19.0— the release whererequire(esm)became available by default in the 20 line.>= 20.6.0would have been an overclaim: below 20.19.0,require()of a transpiled file fails onmaintoo. Bisected across seven 20.x releases.test-engines-floor(20.19.0) andtest-register-fallback(22.18.0, 24.7.0). pnpm needs >= 22.13 and cannot start on 20, so the floor job installs under Node.js 22 and runsintegrate-moduleunder 20.19.0.conditions.includes("require"), makeLoadContext.formatanOptionload()inhooks.mjs;LoadContextinsrc/lib.rsoxc_resolved_path_to_url, or keep callingnext_resolve%is escaped as well, andcreate_resolvepercent-decodes on the way inpathskeeps winningresolve()callscreateResolvefirst;add_short_circuitis unchangednextResolvefirstrequire()path is Node.js', so there is no oxc-node resolution to orderResolvers::resolver()insrc/lib.rs, reference counted rather than leakedpackages/integrate-vitest/__tests__/loader.spec.ts, 54 specs across both loadersThe follow-up you suggested for the three performance changes is not part of this PR; they are simply removed here, and can come back with the benchmarks and semantics tests you asked for.
Verification
A 24-scenario matrix run against
main's build on 21 Node.js releases from 22.15.0 to 26.8.1, diffed per version:There is no case left where this branch reports something different from
main, other than three wheremainis broken and this branch is not:mainimport { v } from "./x.es6"import { v } from "./dep/mod.ts"(dep is"type": "commonjs")node -r @oxc-node/core/registerresolveSync() is not implementedon some versionsThe full suite passes on 15 of those releases with
OXC_TRANSFORM_ALLboth set and unset (integrate-module21/21,integrate-module-bundler22/22,integrate-vitest53/53; the two specs that assert behaviour only the synchronous hooks can deliver are skipped on the fallback).cargo clippy --all-targets --all-features -- -D warningsis clean andpnpm lintreports only the three warnings already present onmain.packages/integrate-vitest/__tests__/loader.spec.tsadds specs for every finding above plus a table for the version gate, each in a child process with a timeout so a loader that keeps the child alive fails instead of hanging CI.CI gains a
test-register-fallbackjob on Node.js 22.18.0 and 24.7.0 — the last releases in each line below the gate, which no other job covered — and the wasm build job now fails if the committed napi bindings drift, including a generated file that was never committed.Remaining difference from
mainmodule.registerHooks()has a single hook chain and runs the most recently registered hook first, so a hook registered before@oxc-node/core/registerruns after it. The native resolver callsnextResolvewith the URL it resolved, which hid the original specifier from the rest of the chain;resolvenow wrapsnextResolveso the chain is asked about the specifier as written, falling back to the resolved URL only when Node.js cannot resolve it itself. Observing hooks therefore see./dep.tsagain, exactly as undermodule.register().What is left is narrower: oxc-node resolves first, so its URL wins over a redirect from a hook that runs after it. Registering the hook after oxc-node makes the redirect win, on both loaders:
Both directions are pinned by tests and described in the README.
Not included
file://server/share/x.ts) are still unsupported — filed as Windows UNC file URLs are not resolved or generated correctly #744. Identical tomain, and I have no Windows machine to verify a fix on, so I left it rather than guess.useDefineForClassFieldsis inverted) and fix: emit ESM imports for injected helpers in ES modules #743 (injected helpers userequire()in ES modules). Both are independent of this PR.