diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index 99d5286eef3a..d9d02a811208 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -609,16 +609,13 @@ impl<'a> LinkerContext<'a> { let html_import: u32 = unsafe { (*parse_graph).html_imports.server_source_indices.slice()[i] }; // SAFETY: `input_files` SoA is append-only; read-only here. - let path_text = unsafe { - &(*parse_graph).input_files.items_source()[html_import as usize] - .path - .text - }; + let path = + unsafe { (*parse_graph).input_files.items_source()[html_import as usize].path }; // SAFETY: sole `&mut` into the per-target map for this lookup. let source_index: u32 = unsafe { (*parse_graph).path_to_source_index_map(Target::Browser) } - .get(path_text) + .get_path(&path) .unwrap_or_else(|| { panic!("Assertion failed: HTML import file not found in pathToSourceIndexMap"); }); diff --git a/src/bundler/PathToSourceIndexMap.rs b/src/bundler/PathToSourceIndexMap.rs index f0e42df43810..a13166f34c4c 100644 --- a/src/bundler/PathToSourceIndexMap.rs +++ b/src/bundler/PathToSourceIndexMap.rs @@ -1,12 +1,14 @@ use bun_collections::StringHashMap; +use bun_paths::fs::is_file_namespace; use crate::IndexStringMap::IndexInt; /// Abstracts over the two structurally-identical `Path` ports (`bun_paths::fs::Path` /// and `bun_resolver::fs::Path`) so the bundler can key the map with either while -/// the crates converge. Both expose `.text: &[u8]`, which is all we need. +/// the crates converge. pub trait PathLike { fn path_text(&self) -> &[u8]; + fn path_namespace(&self) -> &[u8]; } // `bun_resolver::fs::Path` is now a re-export of `bun_paths::fs::Path` (D090), @@ -16,6 +18,10 @@ impl PathLike for bun_paths::fs::Path<'_> { fn path_text(&self) -> &[u8] { self.text } + #[inline] + fn path_namespace(&self) -> &[u8] { + self.namespace + } } /// The lifetime of the keys are not owned by this map. @@ -32,36 +38,69 @@ pub type Map = StringHashMap; /// `found_existing` + value-ptr together, so we hand-roll a thin shim. pub(crate) type GetOrPutResult<'a> = bun_collections::string_hash_map::GetOrPutResult<'a, IndexInt>; +/// Module identity is `(namespace, text)`. File-namespace entries key on bare +/// `text`; other namespaces key on `len(namespace) as u32 LE ++ namespace ++ text`. impl PathToSourceIndexMap { + #[inline] + fn composite_key(namespace: &[u8], text: &[u8]) -> Vec { + let mut v = Vec::with_capacity(4 + namespace.len() + text.len()); + v.extend_from_slice(&(namespace.len() as u32).to_le_bytes()); + v.extend_from_slice(namespace); + v.extend_from_slice(text); + v + } + pub(crate) fn get_path(&self, path: &impl PathLike) -> Option { - self.get(path.path_text()) + self.get(path.path_namespace(), path.path_text()) } - pub(crate) fn get(&self, text: impl AsRef<[u8]>) -> Option { - self.map.get(text.as_ref()).copied() + pub(crate) fn get(&self, namespace: &[u8], text: &[u8]) -> Option { + if is_file_namespace(namespace) { + self.map.get(text).copied() + } else { + self.map + .get(Self::composite_key(namespace, text).as_slice()) + .copied() + } } - // Takes `&[u8]` (not `impl AsRef<[u8]>`) - // to avoid E0283 inference ambiguity at `.into()` call sites in bundle_v2. pub(crate) fn put( &mut self, + namespace: &[u8], text: &[u8], value: IndexInt, ) -> Result<(), bun_alloc::AllocError> { // PERF: bun_collections::StringHashMap is keyed by `Box<[u8]>`, so we dupe here. // Revisit once StringHashMap gains a borrowed-key variant. - self.map.put(text, value) + if is_file_namespace(namespace) { + self.map.put(text, value) + } else { + self.map + .put(Self::composite_key(namespace, text).as_slice(), value) + } } pub(crate) fn get_or_put( &mut self, - text: impl AsRef<[u8]>, + namespace: &[u8], + text: &[u8], ) -> Result, bun_alloc::AllocError> { // PERF: see note in `put` re: key duplication. - self.map.get_or_put(text.as_ref()) + if is_file_namespace(namespace) { + self.map.get_or_put(text) + } else { + self.map + .get_or_put(Self::composite_key(namespace, text).as_slice()) + } } - pub fn remove(&mut self, text: impl AsRef<[u8]>) -> bool { - self.map.remove(text.as_ref()).is_some() + pub fn remove(&mut self, namespace: &[u8], text: &[u8]) -> bool { + if is_file_namespace(namespace) { + self.map.remove(text).is_some() + } else { + self.map + .remove(Self::composite_key(namespace, text).as_slice()) + .is_some() + } } } diff --git a/src/bundler/barrel_imports.rs b/src/bundler/barrel_imports.rs index 90d59d5bf52a..6bafef9e39dd 100644 --- a/src/bundler/barrel_imports.rs +++ b/src/bundler/barrel_imports.rs @@ -321,6 +321,7 @@ fn resolve_barrel_records( ); let source = core::mem::take(&mut this.graph.input_files.items_source_mut()[idx]); let source_path: &'static [u8] = source.path.text; + let source_namespace: &'static [u8] = source.path.namespace; let resolve_result = this.resolve_import_records(&mut ResolveImportRecordCtx { import_records: &mut barrel_ir, @@ -338,6 +339,7 @@ fn resolve_barrel_records( PatchImportRecordsCtx { source_index: Index::init(barrel_idx), source_path, + source_namespace, loader, target, force_save: true, @@ -422,9 +424,10 @@ pub(crate) fn schedule_barrel_deferred_imports( // surviving record's path.text becomes the resolved absolute path. // named_imports entries created for the dedup'd record still point at // its index, so the direct path lookup below fails for those entries. - // Build a fallback: raw specifier → surviving record's resolved path - // text, using non-unused records in this file. See #28886. - let mut dedup_fallback: StringArrayHashMap<&'static [u8]> = StringArrayHashMap::default(); + // Build a fallback: raw specifier → surviving record's resolved path, + // using non-unused records in this file. See #28886. + let mut dedup_fallback: StringArrayHashMap> = + StringArrayHashMap::default(); if dev_handle.is_some() { for ir_probe in file_import_records.as_slice() { if ir_probe.flags.contains(import_record::Flags::IS_UNUSED) @@ -438,7 +441,7 @@ pub(crate) fn schedule_barrel_deferred_imports( if ir_probe.original_path == ir_probe.path.text { continue; } - dedup_fallback.put(ir_probe.original_path, ir_probe.path.text)?; + dedup_fallback.put(ir_probe.original_path, ir_probe.path)?; } } @@ -456,18 +459,15 @@ pub(crate) fn schedule_barrel_deferred_imports( // For dedup'd HMR records (is_unused), fall back to a sibling's // resolved path text since the record itself still has the raw // specifier in path.text. - let resolved_path_text = if ir.flags.contains(import_record::Flags::IS_UNUSED) { - dedup_fallback - .get(ir.path.text) - .copied() - .unwrap_or(ir.path.text) + let resolved_path = if ir.flags.contains(import_record::Flags::IS_UNUSED) { + dedup_fallback.get(ir.path.text).copied().unwrap_or(ir.path) } else { - ir.path.text + ir.path }; let target = if ir.source_index.is_valid() { ir.source_index.get() } else if let Some(map) = path_to_source_index_map { - match map.get().get(resolved_path_text) { + match map.get().get_path(&resolved_path) { Some(t) => t, None => continue, } @@ -489,7 +489,7 @@ pub(crate) fn schedule_barrel_deferred_imports( } // Persist the export request on DevServer so it survives across builds. if let Some(dev) = dev_handle { - persist_barrel_export(&dev, resolved_path_text, alias); + persist_barrel_export(&dev, resolved_path.text, alias); } } } @@ -543,18 +543,15 @@ pub(crate) fn schedule_barrel_deferred_imports( continue; } let ir = &file_import_records.as_slice()[ni.import_record_index as usize]; - let resolved_path_text = if ir.flags.contains(import_record::Flags::IS_UNUSED) { - dedup_fallback - .get(ir.path.text) - .copied() - .unwrap_or(ir.path.text) + let resolved_path = if ir.flags.contains(import_record::Flags::IS_UNUSED) { + dedup_fallback.get(ir.path.text).copied().unwrap_or(ir.path) } else { - ir.path.text + ir.path }; let ir_target = if ir.source_index.is_valid() { ir.source_index.get() } else if let Some(map) = path_to_source_index_map { - match map.get().get(resolved_path_text) { + match map.get().get_path(&resolved_path) { Some(t) => t, None => continue, } diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 648bb40bcf38..d0af2320cb67 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -2089,7 +2089,7 @@ pub mod bv2_impl { let secondary_path: &[u8] = &secondary_paths[source_index as usize]; if !secondary_path.is_empty() { let Some(secondary_source_index) = - path_to_source_index_map.get(secondary_path) + path_to_source_index_map.get(b"", secondary_path) else { continue; }; @@ -2134,7 +2134,7 @@ pub mod bv2_impl { let (found_existing, value_ptr): (bool, *mut u32) = { let entry = self .path_to_source_index_map(target) - .get_or_put(path_primary.text) + .get_or_put(path_primary.namespace, path_primary.text) .expect("oom"); ( entry.found_existing, @@ -2374,7 +2374,10 @@ pub mod bv2_impl { // borrowck: get-then-put (instead of a single get-or-put) so the map // borrow doesn't span `enqueue_parse_task` (which needs `&mut self`). - if let Some(existing) = self.path_to_source_index_map(target).get(path.text) { + if let Some(existing) = self + .path_to_source_index_map(target) + .get(path.namespace, path.text) + { out_source_index = Some(Index::init(existing)); } else { path = self @@ -2414,7 +2417,7 @@ pub mod bv2_impl { ) .expect("oom"); self.path_to_source_index_map(target) - .put(path.text, idx) + .put(path.namespace, path.text, idx) .expect("oom"); out_source_index = Some(Index::init(idx)); @@ -2450,11 +2453,11 @@ pub mod bv2_impl { _ => (Target::Browser, Target::ServerComponentsSsr), }; self.path_to_source_index_map(ta) - .put(&key_text, idx) + .put(path.namespace, &key_text, idx) .expect("oom"); if separate_ssr { self.path_to_source_index_map(tb) - .put(&key_text, idx) + .put(path.namespace, &key_text, idx) .expect("oom"); } } @@ -2478,7 +2481,7 @@ pub mod bv2_impl { // borrow doesn't span the resolver / `&mut self` calls below. if self .path_to_source_index_map(target) - .get(path_slice) + .get(b"", path_slice) .is_some() { return Ok(()); @@ -2504,7 +2507,7 @@ pub mod bv2_impl { // `pretty`. result.path_pair.primary = path; self.path_to_source_index_map(target) - .put(path_slice, source_index.get()) + .put(b"", path_slice, source_index.get()) .expect("oom"); let _ = self.graph.ast.append(JSAst::empty_in(self.graph.heap)); // OOM/capacity: fire-and-forget @@ -2572,7 +2575,7 @@ pub mod bv2_impl { // borrowck: get-then-put instead of a single get-or-put. if self .path_to_source_index_map(target) - .get(path.text) + .get(path.namespace, path.text) .is_some() { return Ok(None); @@ -2612,7 +2615,7 @@ pub mod bv2_impl { *p = path; } self.path_to_source_index_map(target) - .put(path.text, source_index.get()) + .put(path.namespace, path.text, source_index.get()) .expect("oom"); let _ = self.graph.ast.append(JSAst::empty_in(self.graph.heap)); // OOM/capacity: fire-and-forget @@ -3136,7 +3139,7 @@ pub mod bv2_impl { // try this.graph.entry_points.append(arena, Index.runtime); let _ = self.graph.ast.append(JSAst::empty_in(self.graph.heap)); // OOM/capacity: fire-and-forget self.path_to_source_index_map(self.transpiler.options.target) - .put(&b"bun:wrap"[..], Index::RUNTIME.get()) + .put(b"", b"bun:wrap", Index::RUNTIME.get()) .expect("oom"); // SAFETY: arena (`self.graph.heap`) outlives the bundle pass; coerce the // `&mut ParseTask` to `*mut` immediately so the `&self` borrow from @@ -4582,7 +4585,7 @@ pub mod bv2_impl { let (value_ptr, found_existing) = { let existing = this .path_to_source_index_map(resolve.import_record.original_target) - .get_or_put(path.text) + .get_or_put(path.namespace, path.text) .expect("oom"); ( std::ptr::from_mut(existing.value_ptr), @@ -6004,8 +6007,9 @@ pub mod bv2_impl { }); import_record.loader = Some(import_record_loader); - if let Some(id) = - self.path_to_source_index_map(target).get(path_primary.text) + if let Some(id) = self + .path_to_source_index_map(target) + .get(path_primary.namespace, path_primary.text) { import_record.source_index = Index::init(id); continue; @@ -6373,7 +6377,10 @@ pub mod bv2_impl { && target.is_server_side() && self.dev_server.is_none(); - if let Some(id) = self.path_to_source_index_map(target).get(path.text) { + if let Some(id) = self + .path_to_source_index_map(target) + .get(path.namespace, path.text) + { if self.dev_server.is_some() && loader != Loader::Html { import_record.path = self.graph.input_files.items_source()[id as usize].path; @@ -6486,7 +6493,7 @@ pub mod bv2_impl { } else { self.graph.path_to_source_index_map(target) }; - let existing = map.get_or_put(key).expect("oom"); + let existing = map.get_or_put(value.path.namespace, key).expect("oom"); ( existing.found_existing, std::ptr::from_mut::(existing.value_ptr), @@ -6586,6 +6593,7 @@ pub mod bv2_impl { pub struct PatchImportRecordsCtx<'a> { pub(crate) source_index: Index, pub(crate) source_path: &'a [u8], + pub(crate) source_namespace: &'a [u8], pub(crate) loader: Loader, pub(crate) target: options::Target, pub(crate) redirect_import_record_index: u32, @@ -6599,6 +6607,7 @@ pub mod bv2_impl { Self { source_index: Index::INVALID, source_path: b"", + source_namespace: b"", loader: Loader::File, target: Target::Browser, redirect_import_record_index: u32::MAX, @@ -6656,7 +6665,11 @@ pub mod bv2_impl { if let Some(compare) = get_redirect_id(ctx.redirect_import_record_index) { if compare == i as u32 { - let _ = path_to_source_index_map.put(ctx.source_path, source_index); // OOM-only Result + let _ = path_to_source_index_map.put( + ctx.source_namespace, + ctx.source_path, + source_index, + ); // OOM-only Result } } } @@ -6748,9 +6761,11 @@ pub mod bv2_impl { let _ = self.graph.ast.append(ast_for_html_entrypoint); // OOM/capacity: fire-and-forget import_record.source_index = Index::init(fake_source_index.0); - let _ = self - .path_to_source_index_map(target) - .put(path_text, fake_source_index.0); // OOM-only Result + let _ = self.path_to_source_index_map(target).put( + path.namespace, + path_text, + fake_source_index.0, + ); // OOM-only Result self.graph .html_imports .server_source_indices @@ -6893,11 +6908,11 @@ pub mod bv2_impl { // Borrowck forbids holding `&input_files.source[i]` while writing // other `input_files` columns through the MultiArrayList accessor // methods (each takes `&mut input_files`), so copy out the - // `'static` path text now and re-borrow `source` per-use below. - let source_path_text: &'static [u8] = this.graph.input_files.items_source() - [result_source_index] - .path - .text; + // `'static` path slices now and re-borrow `source` per-use below. + let (source_path_text, source_namespace): (&'static [u8], &'static [u8]) = { + let p = &this.graph.input_files.items_source()[result_source_index].path; + (p.text, p.namespace) + }; this.source_code_length += if result_source_index != 0 { this.graph.input_files.items_source()[result_source_index] .contents @@ -6966,6 +6981,7 @@ pub mod bv2_impl { PatchImportRecordsCtx { source_index: Index::init(result_source_index as IndexInt), source_path: source_path_text, + source_namespace, loader: result.loader, target: result.ast.target, redirect_import_record_index: result.ast.redirect_import_record_index, @@ -7130,7 +7146,7 @@ pub mod bv2_impl { this.graph .path_to_source_index_map(result_ast_target) - .put(source_path_text, reference_source_index) + .put(source_namespace, source_path_text, reference_source_index) .expect("oom"); this.graph diff --git a/src/paths/lib.rs b/src/paths/lib.rs index b73514d850fa..9afb9f80271e 100644 --- a/src/paths/lib.rs +++ b/src/paths/lib.rs @@ -772,6 +772,12 @@ pub mod fs { } } + /// True when `namespace` is the canonical `file` namespace (empty or `"file"`). + #[inline] + pub fn is_file_namespace(namespace: &[u8]) -> bool { + namespace.is_empty() || namespace == b"file" + } + /// The bundler/resolver's logical /// path (display `pretty`, canonical `text`, `namespace`, parsed `name`). /// @@ -943,7 +949,7 @@ pub mod fs { #[inline] pub fn is_file(&self) -> bool { - self.namespace.is_empty() || self.namespace == b"file" + is_file_namespace(self.namespace) } #[inline] diff --git a/src/runtime/bake/dev_server/incremental_graph.rs b/src/runtime/bake/dev_server/incremental_graph.rs index 3522f6447791..b9c45be098c0 100644 --- a/src/runtime/bake/dev_server/incremental_graph.rs +++ b/src/runtime/bake/dev_server/incremental_graph.rs @@ -1561,7 +1561,7 @@ impl IncrementalGraph { // Clear the cached entry from the path→source-index maps. for map in bv2.graph.build_graphs.values_mut() { - map.remove(abs_path); + map.remove(b"", abs_path); } Ok(()) } diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index c81aa651d166..81237022b5b3 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -338,6 +338,98 @@ describe("bundler", () => { }, }; }); + // Two plugin namespaces that resolve to the same `path` must stay distinct + // modules (esbuild's contract: module identity is (namespace, path)). + itBundled("plugin/NamespaceSamePathDistinctModules", () => { + const loads: string[] = []; + return { + files: { + "index.ts": /* ts */ ` + import a from "nsa:m"; + import b from "nsb:m"; + console.log(JSON.stringify({ a, b })); + `, + }, + plugins(builder) { + builder.onResolve({ filter: /^nsa:/ }, () => ({ path: "SAMEPATH", namespace: "nsa" })); + builder.onResolve({ filter: /^nsb:/ }, () => ({ path: "SAMEPATH", namespace: "nsb" })); + builder.onLoad({ filter: /.*/, namespace: "nsa" }, () => { + loads.push("nsa"); + return { contents: `export default "FROM_NSA";`, loader: "js" }; + }); + builder.onLoad({ filter: /.*/, namespace: "nsb" }, () => { + loads.push("nsb"); + return { contents: `export default "FROM_NSB";`, loader: "js" }; + }); + }, + run: { + stdout: `{"a":"FROM_NSA","b":"FROM_NSB"}`, + }, + onAfterBundle() { + expect(loads.sort()).toEqual(["nsa", "nsb"]); + }, + }; + }); + // A plugin onResolve that returns a `path` equal to a real file on disk but + // with a custom namespace must go through the namespaced onLoad, not read + // the disk file. + itBundled("plugin/NamespaceShadowsDiskFile", ({ root }) => { + let loaded = false; + return { + files: { + "index.ts": /* ts */ ` + import disk from "./real.ts"; + import virt from "virt:real"; + console.log(JSON.stringify({ disk, virt })); + `, + "real.ts": `export default "FROM_DISK";`, + }, + plugins(builder) { + builder.onResolve({ filter: /^virt:real$/ }, () => ({ + path: resolve(root, "real.ts"), + namespace: "virt", + })); + builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { + loaded = true; + return { contents: `export default "FROM_VIRT";`, loader: "js" }; + }); + }, + run: { + stdout: `{"disk":"FROM_DISK","virt":"FROM_VIRT"}`, + }, + onAfterBundle() { + expect(loaded).toBe(true); + }, + }; + }); + // Control for NamespaceSamePathDistinctModules: two specifiers that resolve + // to the SAME (namespace, path) pair are still one module (onLoad fires once, + // a single evaluation). + itBundled("plugin/NamespaceSamePathSameNamespaceDedup", () => { + let loadCount = 0; + return { + files: { + "index.ts": /* ts */ ` + import a from "ns:one"; + import b from "ns:two"; + console.log(a === b, a.tag); + `, + }, + plugins(builder) { + builder.onResolve({ filter: /^ns:/ }, () => ({ path: "SAMEPATH", namespace: "ns" })); + builder.onLoad({ filter: /.*/, namespace: "ns" }, () => { + loadCount++; + return { contents: `export default { tag: "shared" };`, loader: "js" }; + }); + }, + run: { + stdout: "true shared", + }, + onAfterBundle() { + expect(loadCount).toBe(1); + }, + }; + }); itBundled("plugin/ResolveAndLoadNamespaceNested", ({ root }) => { let counter1 = 0; let counter2 = 0;