bundler: port esbuild's "browser" map lookup and bundle a disabled path as an empty module

scripts/browser-field-matrix.ts

@@ -0,0 +1,23 @@
+// <bun> scripts/browser-field-matrix.ts
+// Builds every layout of test/bundler/browser-field-matrix.ts with the bun that runs this script and with esbuild,
+// and prints the layouts where the two disagree about which files an import of a package with a "browser" map loads.
+import { mkdtempSync, realpathSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { bun, esbuild, layouts, table } from "../test/bundler/browser-field-matrix";
+
+const module = await import("esbuild");
+const all = layouts().map(layout => ({ ...layout, name: `${layout.group}: ${layout.name}` }));
+const root = mkdtempSync(join(realpathSync(tmpdir()), "browser-field-matrix-"));
+const [ours, theirs] = [
+  await table(join(root, "bun"), all, bun),
+  await table(join(root, "esbuild"), all, esbuild(module, join(root, "esbuild"))),
+];
+rmSync(root, { recursive: true, force: true });
+
+let same = 0;
+for (const { name } of all) {
+  if (ours[name] === theirs[name]) same++;
+  else console.log(`${name}\n  bun ${Bun.version}:     ${ours[name]}\n  esbuild ${module.version}: ${theirs[name]}`);
+}
+console.log(JSON.stringify({ layouts: all.length, same, differ: all.length - same }));

src/bundler/LinkerContext.rs

@@ -3728,6 +3728,24 @@ impl<'a> LinkerContext<'a> {
             };
         }
 
+        // A named import of the empty module of a disabled file is always `undefined`.
+        if self.parse_graph().input_files.items_source()[other_source_index as usize]
+            .path
+            .is_disabled_module
+            && !alias_is_star
+            && alias.is_some_and(|a| a.slice() != b"default")
+        {
+            return ImportTrackerIterator {
+                value: ImportTracker {
+                    source_index: crate::Index::init(other_source_index),
+                    import_ref: Ref::NONE,
+                    ..Default::default()
+                },
+                status: ImportTrackerStatus::CjsWithoutExports,
+                ..Default::default()
+            };
+        }
+
         let flags = ast_flags[other_id as usize];
 
         // Is this a named import of a file without any exports?
@@ -4016,22 +4034,25 @@ impl<'a> LinkerContext<'a> {
                     if status == ImportTrackerStatus::CjsWithoutExports
                         && !self.is_call_record(prev_source_index, named_import.import_record_index)
                     {
+                        // SAFETY: the symbol slot is disjoint from `named_import` (`graph.ast`).
+                        unsafe { self.graph.symbol_mut(tracker.import_ref) }.import_item_status =
+                            ImportItemStatus::Missing;
                         let source = self.get_source(tracker.source_index.get());
+                        let imported = self.get_source(next_tracker.source_index.get());
                         // SAFETY: `alias` is an arena `*const [u8]` valid for the link pass.
                         let alias = named_import
                             .alias
                             .expect("infallible: alias present")
                             .slice();
-                        // Split-borrow with `named_import` (`&self.graph`) —
-                        // `log_disjoint` returns the disjoint `Transpiler.log` backref.
-                        self.log_disjoint().add_range_warning_fmt(
-                            Some(source),
+                        let message = format_args!(
+                            "Import \"{}\" will always be undefined because the file \"{}\" has no exports",
+                            bstr::BStr::new(alias),
+                            bstr::BStr::new(&imported.path.pretty),
+                        );
+                        self.warn_import_is_undefined(
+                            source,
                             source.range_of_identifier(named_import.alias_loc),
-                            format_args!(
-                                "Import \"{}\" will always be undefined because the file \"{}\" has no exports",
-                                bstr::BStr::new(alias),
-                                bstr::BStr::new(&source.path.pretty),
-                            ),
+                            message,
                         );
                     }
                 }
@@ -4156,6 +4177,17 @@ impl<'a> LinkerContext<'a> {
                             format_args!("Bun's bundler defaults to browser builds instead of node or bun builds. If you want to use node or bun builds, you can set the target to \"node\" or \"bun\" in the transpiler options."),
                             r,
                         );
+                    } else if self
+                        .export_stars_reach_disabled_module(next_tracker.source_index.get())
+                    {
+                        // `export *` from a disabled file gives no name, so the import is `undefined`.
+                        symbol.import_item_status = ImportItemStatus::Missing;
+                        let message = format_args!(
+                            "Import \"{}\" will always be undefined because there is no matching export in \"{}\"",
+                            bstr::BStr::new(alias),
+                            bstr::BStr::new(&next_source.path.pretty),
+                        );
+                        self.warn_import_is_undefined(source, r, message);
                     } else {
                         self.log_disjoint().add_range_error_fmt(
                             Some(source),
@@ -4303,6 +4335,46 @@ impl<'a> LinkerContext<'a> {
         }
     }
 
+    /// Like esbuild, a debug message when the importer is inside node_modules: its user cannot fix it.
+    fn warn_import_is_undefined(
+        &self,
+        source: &Source,
+        range: bun_ast::Range,
+        message: core::fmt::Arguments<'_>,
+    ) {
+        if source.path.is_node_module() {
+            self.log_disjoint()
+                .add_debug_fmt(Some(source), range.loc, message);
+        } else {
+            self.log_disjoint()
+                .add_range_warning_fmt(Some(source), range, message);
+        }
+    }
+
+    /// `source_index` re-exports with `export *`, directly or not, from a disabled module.
+    fn export_stars_reach_disabled_module(&self, source_index: crate::IndexInt) -> bool {
+        let sources = self.parse_graph().input_files.items_source();
+        let import_records = self.graph.ast.items_import_records();
+        let export_stars = self.graph.ast.items_export_star_import_records();
+        let mut visited = vec![source_index];
+        let mut next = 0;
+        while let Some(&file) = visited.get(next) {
+            next += 1;
+            for &record_index in export_stars[file as usize].iter() {
+                let target =
+                    import_records[file as usize].as_slice()[record_index as usize].source_index;
+                if !target.is_valid() || visited.contains(&target.get()) {
+                    continue;
+                }
+                if sources[target.get() as usize].path.is_disabled_module {
+                    return true;
+                }
+                visited.push(target.get());
+            }
+        }
+        false
+    }
+
     /// Is `ref_` the `exports` object of ES module `source_index` (i.e. an
     /// import that resolved here is that module's namespace)?
     /// The record of a named import in `source_index` is an `import()` or a

src/bundler/ParseTask.rs

@@ -278,17 +278,26 @@ impl<'a> ParseTask<'a> {
         ParseTask {
             ctx: Some(ctx_ref),
             path,
-            contents_or_fd: ContentsOrFd::Fd {
-                dir: resolve_result.dirname_fd,
-                file: resolve_result.file_fd,
+            contents_or_fd: if path.is_disabled_module {
+                ContentsOrFd::Contents(b"")
+            } else {
+                ContentsOrFd::Fd {
+                    dir: resolve_result.dirname_fd,
+                    file: resolve_result.file_fd,
+                }
             },
             side_effects: resolve_result.primary_side_effects_data,
             // D042: resolver-side and bundler-side `jsx::Pragma` are the SAME
             // nominal type (`bun_options_types::jsx::Pragma`). Preserves
             // jsxImportSource/runtime/etc. from tsconfig.json.
             jsx: resolve_result.jsx.clone(),
             source_index,
-            module_type: resolve_result.module_type,
+            // The empty module is `module.exports = {}`, also for an ".mjs" file.
+            module_type: if path.is_disabled_module {
+                options::ModuleType::Cjs
+            } else {
+                resolve_result.module_type
+            },
             emit_decorator_metadata: resolve_result.flags.emit_decorator_metadata(),
             experimental_decorators: resolve_result.flags.experimental_decorators(),
             use_define_for_class_fields: resolve_result.flags.use_define_for_class_fields(),
@@ -650,6 +659,7 @@ pub mod parse_worker {
                 pretty: b"",
                 is_disabled: false,
                 is_symlink: false,
+                is_disabled_module: false,
             },
             contents: std::borrow::Cow::Borrowed(runtime_code),
             // `Source.index` is `bun_ast::Index` (newtype `u32`),
@@ -1640,7 +1650,7 @@ pub mod parse_worker {
                 break 'brk false;
             }
 
-            if file_path.namespace == b"node" {
+            if file_path.namespace == b"node" || file_path.is_disabled_module {
                 break 'brk false;
             }
             true

src/bundler/PathToSourceIndexMap.rs

@@ -1,53 +1,73 @@
 use bun_collections::StringHashMap;
+use bun_paths::fs::Path;
 
 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.
-pub trait PathLike {
-    fn path_text(&self) -> &[u8];
-}
-
-// `bun_resolver::fs::Path` is now a re-export of `bun_paths::fs::Path` (D090),
-// so a single impl covers both.
-impl PathLike for bun_paths::fs::Path<'_> {
-    #[inline]
-    fn path_text(&self) -> &[u8] {
-        self.text
-    }
-}
+/// Pretty-path prefix of the empty module of a disabled file, as in esbuild.
+pub(crate) const DISABLED_PRETTY_PREFIX: &[u8] = b"(disabled):";
 
 /// The lifetime of the keys are not owned by this map.
 ///
 /// We assume it's arena allocated.
 #[derive(Default)]
-pub struct PathToSourceIndexMap {
-    pub(crate) map: Map,
+pub struct PathMap<V> {
+    pub(crate) map: StringHashMap<V>,
+    /// A disabled file and its empty module are two modules with one `text`.
+    disabled_modules: StringHashMap<V>,
 }
 
+pub type PathToSourceIndexMap = PathMap<IndexInt>;
+
 pub type Map = StringHashMap<IndexInt>;
 
 /// std `HashMap::entry` doesn't expose
 /// `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>;
+pub(crate) type GetOrPutResult<'a, V = IndexInt> =
+    bun_collections::string_hash_map::GetOrPutResult<'a, V>;
+
+impl<V: Copy + Default> PathMap<V> {
+    fn map_for(&self, path: &Path<'_>) -> &StringHashMap<V> {
+        if path.is_disabled_module {
+            &self.disabled_modules
+        } else {
+            &self.map
+        }
+    }
+
+    fn map_for_mut(&mut self, path: &Path<'_>) -> &mut StringHashMap<V> {
+        if path.is_disabled_module {
+            &mut self.disabled_modules
+        } else {
+            &mut self.map
+        }
+    }
+
+    pub(crate) fn get_path(&self, path: &Path<'_>) -> Option<V> {
+        self.map_for(path).get(path.text).copied()
+    }
+
+    pub(crate) fn put_path(
+        &mut self,
+        path: &Path<'_>,
+        value: V,
+    ) -> Result<(), bun_alloc::AllocError> {
+        self.map_for_mut(path).put(path.text, value)
+    }
 
-impl PathToSourceIndexMap {
-    pub(crate) fn get_path(&self, path: &impl PathLike) -> Option<IndexInt> {
-        self.get(path.path_text())
+    pub(crate) fn get_or_put_path(
+        &mut self,
+        path: &Path<'_>,
+    ) -> Result<GetOrPutResult<'_, V>, bun_alloc::AllocError> {
+        self.map_for_mut(path).get_or_put(path.text)
     }
 
-    pub(crate) fn get(&self, text: impl AsRef<[u8]>) -> Option<IndexInt> {
+    pub(crate) fn get(&self, text: impl AsRef<[u8]>) -> Option<V> {
         self.map.get(text.as_ref()).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,
-        text: &[u8],
-        value: IndexInt,
-    ) -> Result<(), bun_alloc::AllocError> {
+    pub(crate) fn put(&mut self, text: &[u8], value: V) -> 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)
@@ -56,12 +76,25 @@
     pub(crate) fn get_or_put(
         &mut self,
         text: impl AsRef<[u8]>,
-    ) -> Result<GetOrPutResult<'_>, bun_alloc::AllocError> {
+    ) -> Result<GetOrPutResult<'_, V>, bun_alloc::AllocError> {
         // PERF: see note in `put` re: key duplication.
         self.map.get_or_put(text.as_ref())
     }
 
     pub fn remove(&mut self, text: impl AsRef<[u8]>) -> bool {
         self.map.remove(text.as_ref()).is_some()
     }
+
+    pub(crate) fn values(&self) -> impl Iterator<Item = &V> {
+        self.map.values().chain(self.disabled_modules.values())
+    }
+
+    pub(crate) fn reserve(&mut self, additional: usize) {
+        self.map.reserve(additional);
+    }
+
+    pub(crate) fn clear(&mut self) {
+        self.map.clear();
+        self.disabled_modules.clear();
+    }
 }