fix: hand Node.js' null source back as null; cover the load hook's dependency and addon edge cases

.github/workflows/CI.yml

@@ -320,9 +320,15 @@ jobs:
         node:
           - "22"
           - "24"
+          # The synchronous `module.registerHooks()` path only runs from v26.2.0 up, so
+          # without a 26 job no Linux target exercises it.
+          - "26"
         exclude:
           - target: armv7-unknown-linux-gnueabihf
             node: "24"
+          # `node:26-slim` ships no arm/v7 image either.
+          - target: armv7-unknown-linux-gnueabihf
+            node: "26"
     # Node.js on qemu segfaults on s390x and arm64v8 when using 24.04
     # See also https://github.com/actions/runner-images/issues/11471
     runs-on: ${{ contains(matrix.target, 'aarch64') && 'ubuntu-24.04-arm' || 'ubuntu-24.04' }}

packages/core/index.d.ts

@@ -29,8 +29,14 @@ export declare function load(url: string, context: LoadContext, nextLoad: (arg0:
 export interface LoadContext {
   /** Export conditions of the relevant `package.json` */
   conditions?: Array<string>
-  /** The format optionally supplied by the `resolve` hook chain */
-  format: string | null
+  /**
+   * The format optionally supplied by the `resolve` hook chain. Node.js passes it as
+   * `undefined`, not `null`, when the chain reported none — a `.node` or `.wasm` file
+   * resolved without its flag, any extension Node.js does not know — and a required
+   * field would reject the whole context with "Missing field `format`" instead of
+   * letting Node.js raise its own `ERR_UNKNOWN_FILE_EXTENSION`.
+   */
+  format?: string | null
   /** An object whose key-value pairs represent the assertions for the module to import */
   importAttributes: Record<string, string>
 }

packages/core/register.mjs

@@ -103,8 +103,11 @@ function load(url, context, nextLoad) {
   // `.cts` entry gets reported at the transformed position rather than the original one.
   // Asking `oxcLoad` first is what keeps a CommonJS-reported file that actually contains
   // ESM syntax running as an ES module. `commonjs-typescript` — Node.js' own format for a
-  // `.ts` file it strips types from — belongs on that same path, hence the prefix test.
-  if (result.format.startsWith("commonjs")) {
+  // `.ts` file it strips types from — is not deferred, because that translator needs the
+  // source and rejects `null`; it is passed through untouched so Node.js reports its own
+  // error (`ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING`) for a `.ts` dependency instead
+  // of one blaming the hook.
+  if (result.format === "commonjs") {
     // A null source is what `module.register()`'s asynchronous default load returned for
     // every CommonJS module, and it is the one shape that keeps `require()` inside such a
     // module working on every runtime: a source-bearing result made Node.js short-circuit

packages/integrate-vitest/__tests__/register-hooks.spec.ts

@@ -32,6 +32,32 @@ import { afterAll, describe, expect, test } from "vitest";
 const CORE = fileURLToPath(new URL("../../core", import.meta.url));
 const COMMONJS = JSON.stringify({ name: "fx", private: true, type: "commonjs" });
 
+// The loader's own addon is the one binary that is guaranteed to be loadable by the
+// Node.js running these tests; a WASI build of the package has none.
+const addon = readdirSync(CORE).find((name) => name.endsWith(".node"));
+const ADDON_ENTRY = [
+  'import addon from "./addon.node";',
+  'console.log("addon:", typeof addon.transform);',
+].join("\n");
+const ADDON_DEP_ENTRY = [
+  'import addon from "addon-dep";',
+  'console.log("addon:", typeof addon.transform);',
+].join("\n");
+// `café` in latin-1: the trailing `0xe9` is not valid UTF-8, and Node.js decodes it to a
+// single replacement character — a four character string either way.
+const LATIN1_SOURCE = Uint8Array.from([
+  ...Buffer.from('module.exports = "caf', "utf8"),
+  0xe9,
+  ...Buffer.from('";\n', "utf8"),
+]);
+
+const [nodeMajor, nodeMinor] = process.versions.node.split(".", 2).map(Number);
+// `import` of a `.node` addon is on by default from v24.19.0 and v26.5.0; on the 22 line
+// the flag exists (v22.20.0) but stays opt-in. 23 and 25 are end-of-life and not in the
+// CI matrix, so they are not modelled.
+const addonImportsByDefault =
+  (nodeMajor === 24 && nodeMinor >= 19) || nodeMajor > 26 || (nodeMajor === 26 && nodeMinor >= 5);
+
 const roots: string[] = [];
 
 afterAll(() => {
@@ -58,7 +84,11 @@ function fixture(files: Record<string, string | Uint8Array>): string {
   return root;
 }
 
-function run(root: string, args: string[]): string {
+function spawn(
+  root: string,
+  args: string[],
+  env: NodeJS.ProcessEnv = {},
+): { status: number | null; output: string } {
   const result = spawnSync(
     process.execPath,
     // A bare specifier resolves through the symlinked node_modules on every platform,
@@ -73,46 +103,80 @@ function run(root: string, args: string[]): string {
         OXC_LOG: undefined,
         TS_NODE_PROJECT: undefined,
         OXC_TSCONFIG_PATH: undefined,
+        ...env,
       },
       timeout: 30_000,
     },
   );
   const output = `${result.stdout}${result.stderr}`;
   expect(result.error, result.error?.message).toBeFalsy();
-  expect(result.status, output).toBe(0);
+  return { status: result.status, output };
+}
+
+function run(root: string, args: string[], env: NodeJS.ProcessEnv = {}): string {
+  const { status, output } = spawn(root, args, env);
+  expect(status, output).toBe(0);
   return output;
 }
 
-describe("a source that is not UTF-8", () => {
-  // The loader's own addon is the one binary that is guaranteed to be loadable by the
-  // Node.js running these tests; a WASI build of the package has none.
-  const addon = readdirSync(CORE).find((name) => name.endsWith(".node"));
+/** Runs a fixture that must fail, and must fail with Node.js' own error rather than one
+ * blaming the load hook's return shape. */
+function runFailing(root: string, args: string[], code: string, env: NodeJS.ProcessEnv = {}) {
+  const { status, output } = spawn(root, args, env);
+  expect(status, output).not.toBe(0);
+  expect(output).toContain(code);
+  expect(output).not.toContain("ERR_INVALID_RETURN_PROPERTY_VALUE");
+  expect(output).not.toContain("Missing field");
+}
 
+describe("a local source that is not UTF-8", () => {
   test.skipIf(addon === undefined)("an imported `.node` addon still loads", () => {
     // `.node` resolves as `commonjs`, so the addon arrives at the load hook as a binary
     // blob. Transforming it is not possible and not needed: `process.dlopen` reads the
     // file itself once the CommonJS machinery takes over.
     const root = fixture({
       "package.json": COMMONJS,
       "addon.node": readFileSync(join(CORE, addon!)),
-      "entry.mts": [
-        'import addon from "./addon.node";',
+      "entry.mts": ADDON_ENTRY,
+    });
+    expect(run(root, ["./entry.mts"])).toContain("addon: function");
+  });
+
+  test.skipIf(addon === undefined)(
+    "an imported `.node` addon still loads with --experimental-addon-modules",
+    () => {
+      // The flag decides the format Node.js reports for a `.node` file, not the one
+      // oxc-node does: a local addon is still `commonjs` and still reaches `dlopen`
+      // through the CommonJS machinery rather than Node.js' `addon` translator.
+      const root = fixture({
+        "package.json": COMMONJS,
+        "addon.node": readFileSync(join(CORE, addon!)),
+        "entry.mts": ADDON_ENTRY,
+      });
+      expect(run(root, ["--experimental-addon-modules", "./entry.mts"])).toContain(
+        "addon: function",
+      );
+    },
+  );
+
+  test.skipIf(addon === undefined)("a `require()`d `.node` addon still loads", () => {
+    // `require()` reaches the synchronous hooks and is handed straight back to Node.js;
+    // `module.register()` never showed it to the hooks at all.
+    const root = fixture({
+      "package.json": COMMONJS,
+      "addon.node": readFileSync(join(CORE, addon!)),
+      "entry.cts": [
+        'const addon = require("./addon.node");',
         'console.log("addon:", typeof addon.transform);',
       ].join("\n"),
     });
-    expect(run(root, ["./entry.mts"])).toContain("addon: function");
+    expect(run(root, ["./entry.cts"])).toContain("addon: function");
   });
 
   test("an imported latin-1 CommonJS file still loads", () => {
     const root = fixture({
       "package.json": COMMONJS,
-      // `café` in latin-1: the trailing `0xe9` is not valid UTF-8, and Node.js decodes it
-      // to a single replacement character — a four character string either way.
-      "legacy.js": Uint8Array.from([
-        ...Buffer.from('module.exports = "caf', "utf8"),
-        0xe9,
-        ...Buffer.from('";\n', "utf8"),
-      ]),
+      "legacy.js": LATIN1_SOURCE,
       "entry.mts": [
         'import legacy from "./legacy.js";',
         'console.log("latin1:", legacy.length);',
@@ -122,6 +186,100 @@ describe("a source that is not UTF-8", () => {
   });
 });
 
+// A dependency is resolved by oxc-node but its format is left to Node.js, so these are
+// the cases where Node.js' own formats — `addon`, `commonjs-typescript`,
+// `module-typescript`, or none at all — reach the load hook. The CI matrix runs the suite
+// under both `OXC_TRANSFORM_ALL` values; for a dependency that setting decides whether
+// `transform_output` transforms it or hands it back untouched, so these run under both
+// regardless of what the job set.
+describe.each(["false", "true"])("a dependency, OXC_TRANSFORM_ALL=%s", (transformAll) => {
+  const env = { OXC_TRANSFORM_ALL: transformAll };
+  const addonDep = () =>
+    fixture({
+      "package.json": COMMONJS,
+      "node_modules/addon-dep/package.json": JSON.stringify({
+        name: "addon-dep",
+        main: "addon.node",
+      }),
+      "node_modules/addon-dep/addon.node": readFileSync(join(CORE, addon!)),
+      "entry.mts": ADDON_DEP_ENTRY,
+    });
+
+  test.skipIf(addon === undefined)("an addon still loads with --experimental-addon-modules", () => {
+    // Not specific to the synchronous hooks — the asynchronous default load returned the
+    // same `null`. Node.js decides `addon` and hands the load hook `source: null`; its
+    // `addon` translator asserts exactly `null` on the way back, and an `undefined`, which
+    // is what a dropped `Option` serialised to, fails with ERR_INVALID_RETURN_PROPERTY_VALUE.
+    expect(run(addonDep(), ["--experimental-addon-modules", "./entry.mts"], env)).toContain(
+      "addon: function",
+    );
+  });
+
+  test.skipIf(addon === undefined || nodeMajor === 23 || nodeMajor === 25)(
+    "an addon follows Node.js' own default for the flag",
+    () => {
+      // With the flag off Node.js reports no format for the file — `format: undefined`,
+      // which the load context has to accept — so the load hook short-circuits to
+      // `nextLoad` and Node.js' own error is what shows, exactly as without oxc-node.
+      if (addonImportsByDefault) {
+        expect(run(addonDep(), ["./entry.mts"], env)).toContain("addon: function");
+      } else {
+        runFailing(addonDep(), ["./entry.mts"], "ERR_UNKNOWN_FILE_EXTENSION", env);
+      }
+    },
+  );
+
+  test("a latin-1 CommonJS file still loads", () => {
+    // The UTF-8 check in `transform_output` runs before the node_modules skip, so a
+    // dependency is deferred to the CommonJS machinery the same way a local file is.
+    const root = fixture({
+      "package.json": COMMONJS,
+      "node_modules/latin-dep/package.json": JSON.stringify({
+        name: "latin-dep",
+        main: "index.js",
+      }),
+      "node_modules/latin-dep/index.js": LATIN1_SOURCE,
+      "entry.mts": [
+        'import legacy from "latin-dep";',
+        'console.log("latin1:", legacy.length);',
+      ].join("\n"),
+    });
+    expect(run(root, ["./entry.mts"], env)).toContain("latin1: 4");
+  });
+
+  test("a `.cts` file gets Node.js' own type-stripping error, not one blaming the hook", () => {
+    // `.cts` under node_modules is `commonjs-typescript` to Node.js, a format whose
+    // translator needs the source: deferring it with `source: null` like plain `commonjs`
+    // is an invalid return shape. Node.js refuses type stripping in node_modules on every
+    // path, so what has to hold is that *its* error is the one reported.
+    const root = fixture({
+      "package.json": COMMONJS,
+      "node_modules/ts-dep/package.json": JSON.stringify({
+        name: "ts-dep",
+        exports: "./index.cts",
+      }),
+      "node_modules/ts-dep/index.cts": "const c: number = 3;\nexport { c };\n",
+      "entry.mts": ['import { c } from "ts-dep";', 'console.log("cts:", c);'].join("\n"),
+    });
+    runFailing(root, ["./entry.mts"], "ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING", env);
+  });
+
+  test("an `.mts` file gets Node.js' own type-stripping error, not one blaming the hook", () => {
+    // `module-typescript` is the sibling of `commonjs-typescript`; it is never deferred
+    // and has to stay passed through with its source.
+    const root = fixture({
+      "package.json": COMMONJS,
+      "node_modules/mts-dep/package.json": JSON.stringify({
+        name: "mts-dep",
+        exports: "./index.mts",
+      }),
+      "node_modules/mts-dep/index.mts": "export const c: number = 3;\n",
+      "entry.mts": ['import { c } from "mts-dep";', 'console.log("mts:", c);'].join("\n"),
+    });
+    runFailing(root, ["./entry.mts"], "ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING", env);
+  });
+});
+
 // `--conditions` adds to the condition set of every request, so neither `require` nor
 // `import` says anything about which loader asked. Both directions have to keep working
 // with either one of them injected.

src/lib.rs

@@ -1027,8 +1027,12 @@ fn json_format(context: &ResolveContext) -> &'static str {
 pub struct LoadContext {
     /// Export conditions of the relevant `package.json`
     pub conditions: Option<Vec<String>>,
-    /// The format optionally supplied by the `resolve` hook chain
-    pub format: Either<String, Null>,
+    /// The format optionally supplied by the `resolve` hook chain. Node.js passes it as
+    /// `undefined`, not `null`, when the chain reported none — a `.node` or `.wasm` file
+    /// resolved without its flag, any extension Node.js does not know — and a required
+    /// field would reject the whole context with "Missing field `format`" instead of
+    /// letting Node.js raise its own `ERR_UNKNOWN_FILE_EXTENSION`.
+    pub format: Option<Either<String, Null>>,
     /// An object whose key-value pairs represent the assertions for the module to import
     pub import_attributes: HashMap<String, String>,
 }
@@ -1055,7 +1059,9 @@ pub fn load<'env>(
     tracing::debug!(url = ?url, context = ?context, "load");
     if url.starts_with("data:") || {
         match context.format {
-            Either::A(ref format) => format == "builtin" || format == "json" || format == "wasm",
+            Some(Either::A(ref format)) => {
+                format == "builtin" || format == "json" || format == "wasm"
+            }
             _ => true,
         }
     } {
@@ -1194,7 +1200,13 @@ fn transform_output(
                 return Ok(loaded);
             }
             tracing::debug!("No source code to transform {}", url);
-            Ok(LoadFnOutput { format: output.format, source: None, response_url: Some(url) })
+            // The `null` Node.js handed over is handed back as-is: its `addon`
+            // translator asserts `source === null`, and an `undefined` fails it.
+            Ok(LoadFnOutput {
+                format: output.format,
+                source: Some(Either4::D(Null)),
+                response_url: Some(url),
+            })
         }
         Some(Either4::A(_) | Either4::B(_) | Either4::C(_)) => {
             // `url` is a URL, so a `?query` or `#fragment` has to be stripped before it can
@@ -1210,14 +1222,16 @@ fn transform_output(
             // and lets the CommonJS machinery read the file — but the synchronous
             // `defaultLoadSync` behind `module.registerHooks()` always reads it. Drop the
             // bytes and defer the same way instead of failing to decode them.
+            // `commonjs-typescript` is not deferred: Node.js' type-stripping translator
+            // needs the source, and handing it `null` is an invalid return shape.
             if !is_json
-                && output.format.starts_with("commonjs")
+                && output.format == "commonjs"
                 && output.source.as_ref().unwrap().try_as_str().is_err()
             {
                 tracing::debug!("Not UTF-8, deferring to the CommonJS loader {}", url);
                 return Ok(LoadFnOutput {
                     format: output.format,
-                    source: None,
+                    source: Some(Either4::D(Null)),
                     response_url: Some(url),
                 });
             }