diff --git a/src/js_printer/lib.rs b/src/js_printer/lib.rs index 873a5c04e09d..f2851204709a 100644 --- a/src/js_printer/lib.rs +++ b/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>; +/// 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] diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index b56455b6e00f..1ba5bf2d76bb 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/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 diff --git a/test/cli/test/isolation.test.ts b/test/cli/test/isolation.test.ts index 6b1c2b75e431..9c0dc1928b75 100644 --- a/test/cli/test/isolation.test.ts +++ b/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": `