Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/bundler/LinkerContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
Expand Down
61 changes: 50 additions & 11 deletions src/bundler/PathToSourceIndexMap.rs
Original file line number Diff line number Diff line change
@@ -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),
Expand All @@ -16,6 +18,10 @@
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.
Expand All @@ -32,36 +38,69 @@
/// `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`.
Comment thread
robobun marked this conversation as resolved.
impl PathToSourceIndexMap {
#[inline]
fn composite_key(namespace: &[u8], text: &[u8]) -> Vec<u8> {
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<IndexInt> {
self.get(path.path_text())
self.get(path.path_namespace(), path.path_text())
}

pub(crate) fn get(&self, text: impl AsRef<[u8]>) -> Option<IndexInt> {
self.map.get(text.as_ref()).copied()
pub(crate) fn get(&self, namespace: &[u8], text: &[u8]) -> Option<IndexInt> {
if is_file_namespace(namespace) {
self.map.get(text).copied()
} else {
self.map
.get(Self::composite_key(namespace, text).as_slice())
.copied()
}
}

Check warning on line 65 in src/bundler/PathToSourceIndexMap.rs

View check run for this annotation

Claude / Claude Code Review

File-namespace bare key can collide with composite key in same map

The two encoding schemes still share one keyspace non-injectively: a file-namespace bare key can be byte-identical to a composite key (e.g. `{namespace:'file', path:'\x03\x00\x00\x00abcXYZ'}` collides with `{namespace:'abc', path:'XYZ'}`). Since `is_file_namespace` guarantees the composite branch never sees an empty namespace, prefixing file-namespace keys with a reserved `[0,0,0,0]` header (a length `composite_key` never emits) would close this cheaply and stay consistent with the length-prefix
Comment thread
robobun marked this conversation as resolved.

// 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<GetOrPutResult<'_>, 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()
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
35 changes: 16 additions & 19 deletions src/bundler/barrel_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -338,6 +339,7 @@ fn resolve_barrel_records(
PatchImportRecordsCtx {
source_index: Index::init(barrel_idx),
source_path,
source_namespace,
loader,
target,
force_save: true,
Expand Down Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
let mut dedup_fallback: StringArrayHashMap<bun_paths::fs::Path<'static>> =
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)
Expand All @@ -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)?;
}
}

Expand All @@ -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,
}
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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,
}
Expand Down
Loading
Loading