test runner: keep plugin onResolve namespace in cached module records under --isolate

src/js_printer/lib.rs

@@ -67,6 +67,15 @@ use renamer as rename;
 // revisit if profiling shows allocation pressure during link.
 pub type MangledProps = bun_collections::ArrayHashMap<Ref, Box<[u8]>>;
 
+/// The namespace the printed specifier of `record` starts with (`namespace:path`), if any.
+fn printed_namespace(record: &ImportRecord) -> Option<&'static [u8]> {
+    (record
+        .flags
+        .contains(ImportRecordFlags::PRINT_NAMESPACE_IN_PATH)
+        && !record.path.is_file())
+    .then_some(record.path.namespace)
+}
+
 /// js_printer is the sole producer of ModuleInfo records; the bundler/runtime
 /// only consume the serialized form.
 pub mod analyze_transpiled_module {
@@ -700,6 +709,16 @@ pub mod analyze_transpiled_module {
             StringID(idx)
         }
 
+        /// Interns the specifier `print_import_record_path` prints for `record`, so the
+        /// module record requests the same module as the printed source.
+        pub(crate) fn str_for_import_record(&mut self, record: &super::ImportRecord) -> StringID {
+            let path = record.path.text;
+            match super::printed_namespace(record) {
+                Some(namespace) => self.str(&[namespace, b":".as_slice(), path].concat()),
+                None => self.str(path),
+            }
+        }
+
         pub(crate) fn request_module(
             &mut self,
             import_record_path: StringID,
@@ -5602,15 +5621,13 @@ pub(crate) mod __gated_printer {
                         self.print_whitespacer(ws!(b"from "));
                     }
 
-                    let irp = &self.import_record(s.import_record_index as usize).path.text;
-                    self.print_import_record_path(
-                        self.import_record(s.import_record_index as usize),
-                    );
+                    let import_record = self.import_record(s.import_record_index as usize);
+                    self.print_import_record_path(import_record);
                     self.print_semicolon_after_statement();
 
                     if Self::MAY_HAVE_MODULE_INFO {
                         if let Some(mi) = self.module_info() {
-                            let irp_id = mi.str(irp);
+                            let irp_id = mi.str_for_import_record(import_record);
                             mi.request_module(
                                 irp_id,
                                 analyze_transpiled_module::FetchParameters::None,
@@ -5786,7 +5803,6 @@ pub(crate) mod __gated_printer {
                     }
 
                     self.print_whitespacer(ws!(b"} from "));
-                    let irp = &import_record.path.text;
                     self.print_import_record_path(import_record);
                     self.print_semicolon_after_statement();
 
@@ -5795,7 +5811,7 @@ pub(crate) mod __gated_printer {
                         // `name_for_symbol` (which needs `&mut self`) can run between uses.
                         let irp_id = {
                             let mi = self.module_info().expect("infallible: module_info enabled");
-                            let id = mi.str(irp);
+                            let id = mi.str_for_import_record(import_record);
                             mi.request_module(id, analyze_transpiled_module::FetchParameters::None);
                             id
                         };
@@ -6319,11 +6335,10 @@ pub(crate) mod __gated_printer {
                         // reshaped for borrowck — `module_info()` borrows `&mut self`,
                         // so we re-borrow it between `name_for_symbol` calls instead of holding
                         // a single long-lived `mi` across the whole block. `irp_id` is Copy.
-                        let import_record_path = &record.path.text;
                         use analyze_transpiled_module::FetchParameters as FP;
                         let (irp_id, fetch_parameters) = {
                             let mi = self.module_info().expect("infallible: module_info enabled");
-                            let irp_id = mi.str(import_record_path);
+                            let irp_id = mi.str_for_import_record(record);
                             let fetch_parameters: FP = if IS_BUN_PLATFORM {
                                 if let Some(loader) = record.loader {
                                     use bun_ast::Loader;
@@ -6505,21 +6520,13 @@ pub(crate) mod __gated_printer {
             }
 
             let quote = best_quote_char_for_string(import_record.path.text, false);
-            if import_record
-                .flags
-                .contains(ImportRecordFlags::PRINT_NAMESPACE_IN_PATH)
-                && !import_record.path.is_file()
-            {
-                self.print(quote);
-                self.print_string_characters_utf8(import_record.path.namespace, quote);
+            self.print(quote);
+            if let Some(namespace) = printed_namespace(import_record) {
+                self.print_string_characters_utf8(namespace, quote);
                 self.print(b":");
-                self.print_string_characters_utf8(import_record.path.text, quote);
-                self.print(quote);
-            } else {
-                self.print(quote);
-                self.print_string_characters_utf8(import_record.path.text, quote);
-                self.print(quote);
             }
+            self.print_string_characters_utf8(import_record.path.text, quote);
+            self.print(quote);
         }
 
         #[inline]

src/jsc/RuntimeTranspilerCache.rs

@@ -60,7 +60,10 @@ bun_core::declare_scope!(cache, visible);
 /// Version 30: String enum members are stored flat, so folds no longer append onto an inlined member.
 /// Version 31: Standard decorator lowering temporaries have a per-file counter in their name (`_init$1`).
 /// Version 32: Standard decorator lowering keeps class members in place.
-const EXPECTED_VERSION: u32 = 32;
+/// Version 33: The cached ESM record keeps the namespace of an import that a plugin
+/// `onResolve` rewrote (`namespace:path`). Older entries request the bare path, and
+/// the cache-HIT path reinstates #33904 for them.
+const EXPECTED_VERSION: u32 = 33;
 
 /// Source files smaller than this are not written to / read from the on-disk
 /// transpiler cache. Originally 50 KiB, which excluded almost every file in a

test/cli/test/isolation.test.ts

@@ -352,6 +352,122 @@ describe.concurrent("bun test --isolate", () => {
     expect(exitCode).toBe(0);
   });
 
+  // https://github.com/oven-sh/bun/issues/33904
+  // The linker rewrites an import that a plugin onResolve answers. When the answer has a
+  // namespace, the printer emits "namespace:path", and the cached module record has to
+  // request that same specifier. "./data.bar?custom" is moved into a namespace by the
+  // plugin. "virt:thing" is already in one in the source.
+  const pluginNamespaceTestFile = `
+    import { test, expect } from "bun:test";
+    import direct from "./data.bar?custom";
+    import * as star from "./data.bar?custom";
+    import { named as viaClause } from "./reexport-clause.ts";
+    import { named as viaStar } from "./reexport-star.ts";
+    import { ns as viaNamespace } from "./reexport-namespace.ts";
+    import redirected from "./data.bar?redirect";
+    import virtual from "virt:other";
+    import { virtual as viaVirtualReexport } from "./reexport-virtual.ts";
+
+    test("plugin-resolved imports load", () => {
+      expect({
+        direct,
+        star: star.named,
+        viaClause,
+        viaStar,
+        viaNamespace: viaNamespace.named,
+        redirected,
+        virtual,
+        viaVirtualReexport,
+      }).toEqual({
+        direct: "FROM_PLUGIN",
+        star: "FROM_PLUGIN",
+        viaClause: "FROM_PLUGIN",
+        viaStar: "FROM_PLUGIN",
+        viaNamespace: "FROM_PLUGIN",
+        redirected: "REDIRECTED",
+        virtual: "resolved-other",
+        viaVirtualReexport: "resolved-thing",
+      });
+    });
+  `;
+
+  const pluginNamespaceFixture = {
+    "bunfig.toml": `[test]\npreload = ["./plugin.ts"]\n`,
+    "plugin.ts": `
+      import { dirname, resolve } from "node:path";
+      Bun.plugin({
+        name: "query-loader",
+        setup(build) {
+          build.onResolve({ filter: /\\.bar\\?custom$/ }, args => ({
+            path: resolve(dirname(args.importer), args.path.slice(0, -"?custom".length)),
+            namespace: "custom",
+          }));
+          build.onLoad({ filter: /.*/, namespace: "custom" }, () => ({
+            contents: 'export const named = "FROM_PLUGIN"; export default "FROM_PLUGIN";',
+            loader: "js",
+          }));
+          // No namespace: the record stays a plain file path.
+          build.onResolve({ filter: /\\.bar\\?redirect$/ }, args => ({
+            path: resolve(dirname(args.importer), "redirected.ts"),
+          }));
+          build.onResolve({ filter: /.*/, namespace: "virt" }, args => ({
+            path: "resolved-" + args.path,
+            namespace: "virt",
+          }));
+          build.onLoad({ filter: /.*/, namespace: "virt" }, args => ({
+            contents: "export default " + JSON.stringify(args.path) + ";",
+            loader: "js",
+          }));
+        },
+      });
+    `,
+    "data.bar": "unused",
+    "redirected.ts": `export default "REDIRECTED";`,
+    "reexport-clause.ts": `export { named } from "./data.bar?custom";`,
+    "reexport-star.ts": `export * from "./data.bar?custom";`,
+    "reexport-namespace.ts": `export * as ns from "./data.bar?custom";`,
+    "reexport-virtual.ts": `export { default as virtual } from "virt:thing";`,
+    "a.test.ts": pluginNamespaceTestFile,
+    "b.test.ts": pluginNamespaceTestFile,
+  };
+
+  test.each([
+    ["--isolate", ["--isolate"], {}],
+    // One worker takes both files (scale-up gated), so the second file links from the records the first one cached.
+    ["--parallel worker", ["--parallel=2"], { BUN_TEST_PARALLEL_SCALE_MS: "60000" }],
+  ])("cached module records keep the namespace a plugin onResolve gives an import (%s)", async (_, args, env) => {
+    using dir = tempDir("isolate-plugin-namespace", pluginNamespaceFixture);
+    const { stderr, exitCode } = await runTests(String(dir), args, ["./a.test.ts", "./b.test.ts"], {
+      ...bunEnv,
+      ...env,
+    });
+    expect(normalizeBunSnapshot(stderr, dir)).toContain("2 pass");
+    expect(normalizeBunSnapshot(stderr, dir)).toContain("0 fail");
+    expect(exitCode).toBe(0);
+  });
+
+  // The on-disk transpiler cache stores the module record next to the output. reexport-clause.ts
+  // is padded past the 4 KiB floor of that cache, so the second run rebuilds its record from the entry.
+  test("with --isolate, the on-disk transpiler cache keeps that namespace in the stored module record", async () => {
+    using dir = tempDir("isolate-plugin-namespace-disk-cache", {
+      ...pluginNamespaceFixture,
+      "reexport-clause.ts": `export { named } from "./data.bar?custom";\n//${Buffer.alloc(5 * 1024, "f").toString()}\n`,
+    });
+    const cacheDir = join(String(dir), ".cache");
+    const env = {
+      ...bunEnv,
+      BUN_RUNTIME_TRANSPILER_CACHE_PATH: cacheDir,
+      BUN_DEBUG_ENABLE_RESTORE_FROM_TRANSPILER_CACHE: "1",
+    };
+    for (const run of ["cold", "warm"]) {
+      const { stderr, exitCode } = await runTests(String(dir), ["--isolate"], ["./a.test.ts", "./b.test.ts"], env);
+      expect(normalizeBunSnapshot(stderr, dir), run).toContain("2 pass");
+      expect(normalizeBunSnapshot(stderr, dir), run).toContain("0 fail");
+      expect(fs.readdirSync(cacheDir), run).toHaveLength(1);
+      expect(exitCode, run).toBe(0);
+    }
+  });
+
   test("with --isolate, leaked outbound socket is closed before next file", async () => {
     using dir = tempDir("isolate-socket", {
       "a-connect.test.ts": `