From 288a230b1e56bece29fc596db6f850b4942c9bbb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:20:13 +0000 Subject: [PATCH 01/10] bundler: link an import record through the path map only when resolution pointed it at a module `patch_import_record_source_indices` asked the path map about the path text of every import record of a file. The map is keyed by text alone, and a record that did not resolve to a module still holds its specifier: an external, a builtin, a failed require(), an import that waits for an onResolve plugin. A module with the same text was linked in its place, and the build reported success. A plugin that wraps a package printed `var foo_pkg_default = { wrapped: foo_pkg_default };`. `resolve_import_records` now stores a module path through `ImportRecord::set_module_path`, which sets `PATH_IS_MODULE`. The path map answers for a record only when that flag is set (`PathToSourceIndexMap::get_record`). The pass, the export star fallback and the barrel code all use that lookup. The barrel code finds the target of an HMR duplicate through its surviving record and not through its specifier text. The pass does one probe per record that points at a module. Before, it did one per import record. --- src/ast/import_record.rs | 17 ++ src/bundler/PathToSourceIndexMap.rs | 28 +-- src/bundler/barrel_imports.rs | 84 +++---- src/bundler/bundle_v2.rs | 42 ++-- test/bundler/bundler_barrel.test.ts | 49 ++++ test/bundler/bundler_edgecase.test.ts | 19 ++ test/bundler/bundler_plugin.test.ts | 329 ++++++++++++++++++++++++++ 7 files changed, 486 insertions(+), 82 deletions(-) diff --git a/src/ast/import_record.rs b/src/ast/import_record.rs index aa4160ec68c4..a8d421104730 100644 --- a/src/ast/import_record.rs +++ b/src/ast/import_record.rs @@ -107,6 +107,23 @@ bitflags::bitflags! { /// `IS_UNUSED` for its own reasons (an unused TypeScript import, a macro /// import), and those records must never be resolved. const IS_BARREL_DEFERRED = 1 << 18; + + /// The bundler resolved this record to a module and stored that + /// module's path in `path` (`ImportRecord::set_module_path`). Without + /// this flag `path` can still be the specifier (an external, a builtin, + /// a failed or pending resolve), and a module can have the same text. + /// Only valid while the bundler scans: the linker repoints `path` later. + const PATH_IS_MODULE = 1 << 19; + } +} + +impl ImportRecord { + /// Point `path` at the module this record resolved to. The bundler's path + /// map pass sets `source_index` from it. + #[inline] + pub fn set_module_path(&mut self, path: Path<'static>) { + self.path = path; + self.flags.insert(Flags::PATH_IS_MODULE); } } diff --git a/src/bundler/PathToSourceIndexMap.rs b/src/bundler/PathToSourceIndexMap.rs index f0e42df43810..edb6fc381af1 100644 --- a/src/bundler/PathToSourceIndexMap.rs +++ b/src/bundler/PathToSourceIndexMap.rs @@ -1,23 +1,8 @@ +use bun_ast::{ImportRecord, ImportRecordFlags}; use bun_collections::StringHashMap; 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 - } -} - /// The lifetime of the keys are not owned by this map. /// /// We assume it's arena allocated. @@ -33,8 +18,15 @@ pub type Map = StringHashMap; pub(crate) type GetOrPutResult<'a> = bun_collections::string_hash_map::GetOrPutResult<'a, IndexInt>; impl PathToSourceIndexMap { - pub(crate) fn get_path(&self, path: &impl PathLike) -> Option { - self.get(path.path_text()) + /// The module `record` resolved to. `None` unless resolution stored that + /// module's path in `record.path`: any other path text is a specifier, and + /// a module with the same text is a different thing. + #[inline] + pub(crate) fn get_record(&self, record: &ImportRecord) -> Option { + if !record.flags.contains(ImportRecordFlags::PATH_IS_MODULE) { + return None; + } + self.get(record.path.text) } pub(crate) fn get(&self, text: impl AsRef<[u8]>) -> Option { diff --git a/src/bundler/barrel_imports.rs b/src/bundler/barrel_imports.rs index 39715de37b48..9a322a8c454d 100644 --- a/src/bundler/barrel_imports.rs +++ b/src/bundler/barrel_imports.rs @@ -323,7 +323,25 @@ fn record_target( if rec.source_index.is_valid() { return Some(rec.source_index.get()); } - map?.get_path(&rec.path) + map?.get_record(rec) +} + +/// The record that holds the target of `ir`. ConvertESMExportsForHmr marks a +/// duplicate import of one specifier as unused, so it is never resolved: its +/// target is the one of the surviving record (`dedup_fallback`, keyed by +/// specifier). See #28886. +#[inline] +fn surviving_record<'a>( + records: &'a [bun_ast::ImportRecord], + dedup_fallback: &StringArrayHashMap, + ir: &'a bun_ast::ImportRecord, +) -> &'a bun_ast::ImportRecord { + if ir.flags.contains(import_record::Flags::IS_UNUSED) { + if let Some(&survivor) = dedup_fallback.get(ir.path.text) { + return &records[survivor as usize]; + } + } + ir } /// The export names the result of `import()` / `require()` record `idx` can @@ -482,15 +500,14 @@ pub(crate) fn schedule_barrel_deferred_imports( // In HMR, ConvertESMExportsForHmr deduplicates import records by path: // two `import { X } from 'mod'` statements become one, and the second // record is marked is_unused=true. resolveImportRecords then skips those - // records, so their path.text stays as the raw specifier while the - // 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(); + // records, so they never get a target of their own. named_imports entries + // created for the dedup'd record still point at its index. Build a + // fallback: raw specifier → index of the surviving record, using the + // non-unused records in this file that resolved to a module. See + // `surviving_record`. + let mut dedup_fallback: StringArrayHashMap = StringArrayHashMap::default(); if dev_handle.is_some() { - for ir_probe in file_import_records.as_slice() { + for (idx, ir_probe) in file_import_records.as_slice().iter().enumerate() { if ir_probe.flags.contains(import_record::Flags::IS_UNUSED) || ir_probe.flags.contains(import_record::Flags::IS_INTERNAL) { @@ -499,10 +516,14 @@ pub(crate) fn schedule_barrel_deferred_imports( if ir_probe.original_path.is_empty() { continue; } - if ir_probe.original_path == ir_probe.path.text { + if !ir_probe.source_index.is_valid() + && !ir_probe + .flags + .contains(import_record::Flags::PATH_IS_MODULE) + { continue; } - dedup_fallback.put(ir_probe.original_path, ir_probe.path.text)?; + dedup_fallback.put(ir_probe.original_path, idx as u32)?; } } @@ -522,25 +543,8 @@ pub(crate) fn schedule_barrel_deferred_imports( // path map as a read-only fallback. Do NOT write back to the import // record — the dev server intentionally leaves source_indices unset // and other code (IncrementalGraph, printer) depends on that. - // 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) - } else { - ir.path.text - }; - 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) { - Some(t) => t, - None => continue, - } - } else { + let ir = surviving_record(file_import_records.as_slice(), &dedup_fallback, ir); + let Some(target) = record_target(ir, path_to_source_index_map) else { continue; }; @@ -558,7 +562,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, ir.path.text, alias); } } } @@ -618,22 +622,8 @@ pub(crate) fn schedule_barrel_deferred_imports( if matches!(ir.kind, ImportKind::Require | ImportKind::Dynamic) { continue; } - 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) - } else { - ir.path.text - }; - 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) { - Some(t) => t, - None => continue, - } - } else { + let ir = surviving_record(file_import_records.as_slice(), &dedup_fallback, ir); + let Some(ir_target) = record_target(ir, path_to_source_index_map) else { continue; }; diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 6ed38562e915..fe538b4247ab 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -6795,8 +6795,9 @@ pub mod bv2_impl { resolve_queue.get_or_put(path_primary.text).expect("oom"); if resolve_entry.found_existing { // SAFETY: arena-allocated `ParseTask` stored in the queue; arena outlives the pass. - import_record.path = - path_as_static(&unsafe { &**resolve_entry.value_ptr }.path); + import_record.set_module_path(path_as_static( + &unsafe { &**resolve_entry.value_ptr }.path, + )); continue; } @@ -6810,7 +6811,7 @@ pub mod bv2_impl { self.arena().alloc_slice_copy(path_primary.text), ) }; - import_record.path = path_as_static(&path_primary); + import_record.set_module_path(path_as_static(&path_primary)); let _ = path_primary.text; // key already interned by get_or_put bun_core::scoped_log!( Bundle, @@ -7101,11 +7102,9 @@ pub mod bv2_impl { let hash = dev_server .asset_hash(path.text) .expect("cached asset not found"); - import_record.path.text = path.text; - import_record.path.namespace = b"file"; // SAFETY: `alloc_str` returns into the bundler arena which // outlives this `ImportRecord`. See `interned_slice` contract. - import_record.path.pretty = unsafe { + let pretty = unsafe { interned_slice( self.arena() .alloc_str(&format!( @@ -7119,15 +7118,21 @@ pub mod bv2_impl { .as_bytes(), ) }; - import_record.path.is_disabled = false; + import_record.set_module_path(Fs::Path { + text: path.text, + namespace: b"file", + pretty, + is_disabled: false, + ..import_record.path + }); } else { import_record.path.text = path.text; import_record.path.pretty = rel; - import_record.path = path_as_static( + import_record.set_module_path(path_as_static( &self .path_with_pretty_initialized(path, target) .expect("oom"), - ); + )); if loader == Loader::Html || entry.kind == bake_types::CacheKind::Css { @@ -7168,8 +7173,9 @@ pub mod bv2_impl { if let Some(id) = self.path_to_source_index_map(target).get(path.text) { if self.dev_server.is_some() && loader != Loader::Html { - import_record.path = - self.graph.input_files.items_source()[id as usize].path; + import_record.set_module_path( + self.graph.input_files.items_source()[id as usize].path, + ); } else { import_record.source_index = Index::init(id); } @@ -7183,8 +7189,9 @@ pub mod bv2_impl { let resolve_entry = resolve_queue.get_or_put(path.text).expect("oom"); if resolve_entry.found_existing { // SAFETY: arena-allocated `ParseTask` stored in the queue; arena outlives the pass. - import_record.path = - path_as_static(&unsafe { &**resolve_entry.value_ptr }.path); + import_record.set_module_path(path_as_static( + &unsafe { &**resolve_entry.value_ptr }.path, + )); continue; } @@ -7192,7 +7199,7 @@ pub mod bv2_impl { .path_with_pretty_initialized(path, target) .expect("oom"); - import_record.path = path_as_static(path); + import_record.set_module_path(path_as_static(path)); // key already interned by get_or_put — no key_ptr on StringHashMapGetOrPut bun_core::scoped_log!(Bundle, "created ParseTask: {}", bstr::BStr::new(&path.text)); // Arena-owned. @@ -7399,7 +7406,8 @@ pub mod bv2_impl { impl<'a> BundleV2<'a> { /// Patch source_index on import records from pathToSourceIndexMap and /// resolve_tasks_waiting_for_import_source_index. Called after - /// processResolveQueue has registered new modules. + /// processResolveQueue has registered new modules. The map is only asked + /// about records that `resolve_import_records` pointed at a module. pub(crate) fn patch_import_record_source_indices( &mut self, import_records: &mut import_record::List, @@ -7440,7 +7448,7 @@ pub mod bv2_impl { if !only_selected_record(ctx.only_records, i) { continue; } - if let Some(source_index) = path_to_source_index_map.get_path(&record.path) { + if let Some(source_index) = path_to_source_index_map.get_record(record) { if save_import_record_source_index || input_file_loaders[source_index as usize].is_css() { @@ -7779,7 +7787,7 @@ pub mod bv2_impl { let resolved_index = if star_ir.source_index.is_valid() { star_ir.source_index.get() } else if let Some(idx) = - this.graph.build_graphs[result_ast_target].get_path(&star_ir.path) + this.graph.build_graphs[result_ast_target].get_record(star_ir) { idx } else { diff --git a/test/bundler/bundler_barrel.test.ts b/test/bundler/bundler_barrel.test.ts index 5cb287c7b1b2..c5f31427c416 100644 --- a/test/bundler/bundler_barrel.test.ts +++ b/test/bundler/bundler_barrel.test.ts @@ -1626,6 +1626,55 @@ describe("bundler", () => { }; }); + // A re-export that the `external` option keeps external has no module. When + // a late importer un-defers it, it must not be linked to a plugin module whose + // path text is also "react". late.js is loaded only after the "virt" answer is + // queued, so that module is in the path map by then. + itBundled("barrel/ExternalReExportNotLinkedToModuleWithSameText", () => { + const virtAnswered = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import { A } from 'textlib'; + import { x } from 'virt'; + import late from './late.js'; + console.log(A, x, late); + `, + "/late.js": ``, + "/node_modules/textlib/package.json": JSON.stringify({ + name: "textlib", + main: "./index.js", + sideEffects: false, + }), + "/node_modules/textlib/index.js": /* js */ ` + export { default as React } from 'react'; + export { A } from './a.js'; + `, + "/node_modules/textlib/a.js": /* js */ ` + export const A = "a"; + `, + "/node_modules/react/package.json": JSON.stringify({ name: "react", main: "./index.js" }), + "/node_modules/react/index.js": `module.exports = "real";`, + }, + outfile: "/out.js", + external: ["react"], + plugins(builder) { + builder.onResolve({ filter: /^virt$/ }, () => { + virtAnswered.resolve(); + return { path: "react", namespace: "virt" }; + }); + builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { + return { contents: `export const x = "x";`, loader: "js" }; + }); + builder.onLoad({ filter: /late\.js$/ }, async () => { + await virtAnswered.promise; + return { contents: `import { React } from 'textlib'; export default React;`, loader: "js" }; + }); + }, + run: { stdout: "a x real" }, + }; + }); + // When the plugin does not answer, the record falls back to the resolver and // the failure is reported. Resolving the barrel again reported it twice. itBundled("barrel/UnresolvableReExportReportedOnce", { diff --git a/test/bundler/bundler_edgecase.test.ts b/test/bundler/bundler_edgecase.test.ts index 1e421e3b7038..7ba9ba960064 100644 --- a/test/bundler/bundler_edgecase.test.ts +++ b/test/bundler/bundler_edgecase.test.ts @@ -1823,6 +1823,25 @@ describe("bundler", () => { api.expectFile("/out.js").toContain("h("); }, }); + // b.js imports a.js by its absolute path, and the `external` option names that path. a.js is also + // an entry point, so the bundle has a module with the same path text. The import stays external. + for (const backend of ["cli", "api"] as const) { + itBundled(`edgecase/ExternalFileIsAlsoAnEntryPoint_${backend}`, ({ root }) => ({ + files: { + "/a.js": `export const A = "A_BODY";`, + "/b.js": `import { A } from ${JSON.stringify(join(root, "a.js"))}; console.log(A);`, + }, + entryPoints: ["/a.js", "/b.js"], + external: [join(root, "a.js")], + outdir: "/out", + backend, + onAfterBundle(api) { + const b = api.readFile("/out/b.js"); + expect(b).toMatch(/import \{ A \} from "[^"]*a\.js";/); + expect(b).not.toContain("A_BODY"); + }, + })); + } itBundled("edgecase/IntegerUnderflow#12547", { files: { "/entry.js": ` diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index 7535d6d374c1..51517d0ffefa 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -540,6 +540,335 @@ describe("bundler", () => { }, }); } + + // The bundler's path map is keyed by path text alone. A plugin module can + // have the same text as the specifier of an import that did not resolve to a + // module: an external, a builtin, a failed require(), an import that waits for + // a plugin. Each `SameTextModule` test gives a plugin module that text. The + // import must never be linked to that module. + const realFooPkg = { + "/node_modules/foo-pkg/package.json": JSON.stringify({ name: "foo-pkg", main: "./index.js" }), + "/node_modules/foo-pkg/index.js": `module.exports = "real";`, + }; + + // A wrapper for a package: "wrapped-foo" resolves to a plugin module with the + // path text "foo-pkg", and that module imports the real, external "foo-pkg". + // Linked by text, the wrapper imports itself. + for (const { name, external, answer } of [ + // No plugin matches the wrapper's import. The `external` option decides it. + { name: "ExternalOption", external: ["foo-pkg"] }, + // The plugin declines the wrapper's import. The `external` option decides it. + { name: "PluginDeclines", external: ["foo-pkg"], answer: () => undefined }, + // The plugin answers external for the wrapper's import. + { name: "PluginAnswersExternal", answer: (args: { path: string }) => ({ path: args.path, external: true }) }, + ] as { name: string; external?: string[]; answer?: (args: { path: string }) => object | undefined }[]) { + itBundled(`plugin/SameTextModule_WrappedPackage_${name}`, { + files: { + "/entry.js": /* js */ ` + import wrapper from "wrapped-foo"; + console.log(JSON.stringify(wrapper)); + `, + ...realFooPkg, + }, + external, + plugins(builder) { + builder.onResolve({ filter: /^wrapped-foo$/ }, () => { + return { path: "foo-pkg", namespace: "wrap" }; + }); + if (answer) builder.onResolve({ filter: /^foo-pkg$/ }, answer as never); + builder.onLoad({ filter: /.*/, namespace: "wrap" }, () => { + return { contents: `import real from "foo-pkg"; export default { wrapped: real };`, loader: "js" }; + }); + }, + run: { stdout: `{"wrapped":"real"}` }, + }); + } + + // late.js is loaded only after the "virt" answer is queued. So the plugin + // module is in the path map before the bundler looks at the import in late.js. + for (const { name, text, late, options, setup, stdout } of [ + { + name: "ExternalOption", + text: "react", + late: `import React from "react"; export default React;`, + options: { external: ["react"] }, + stdout: "x real", + }, + { + name: "PackagesExternal", + text: "react", + late: `import React from "react"; export default React;`, + options: { packages: "external" }, + stdout: "x real", + }, + { + // The plugin's answer arrives after the bundler looked at the import. + name: "PluginAnswersExternal", + text: "react", + late: `import React from "react"; export default React;`, + setup: builder => { + builder.onResolve({ filter: /^react$/ }, args => ({ path: args.path, external: true })); + }, + stdout: "x real", + }, + { + // With target "bun", "node:fs" is a builtin and its path text is "fs". + name: "NodeBuiltin", + text: "fs", + late: `import * as fs from "node:fs"; export default typeof fs.existsSync;`, + options: { target: "bun" }, + stdout: "x function", + }, + { + // A require() that fails inside try/catch is not a build error. It throws when it runs. + name: "FailedRequireInTryCatch", + text: "not-installed", + late: `let r; try { r = require("not-installed"); } catch { r = "caught"; } export default r;`, + stdout: "x caught", + }, + ] as { + name: string; + text: string; + late: string; + options?: Partial[1]>; + setup?: (builder: Bun.PluginBuilder) => void; + stdout: string; + }[]) { + itBundled(`plugin/SameTextModule_${name}`, () => { + const virtAnswered = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import { x } from "virt"; + import late from "./late.js"; + console.log(x, late); + `, + "/late.js": ``, + "/node_modules/react/package.json": JSON.stringify({ name: "react", main: "./index.js" }), + "/node_modules/react/index.js": `module.exports = "real";`, + }, + ...options, + plugins(builder) { + builder.onResolve({ filter: /^virt$/ }, () => { + virtAnswered.resolve(); + return { path: text, namespace: "virt" }; + }); + builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { + return { contents: `export const x = "x"; export default "virt";`, loader: "js" }; + }); + setup?.(builder); + builder.onLoad({ filter: /late\.js$/ }, async () => { + await virtAnswered.promise; + return { contents: late, loader: "js" }; + }); + }, + run: { stdout }, + }; + }); + } + + // The same inside one namespace: late.js imports x.js by its absolute path, + // which is the path map's key for the bundled x.js, and a plugin answers + // external for that import. late.js is loaded only after the plugin declined + // the import of x.js in entry.js, so x.js is in the map by then. + itBundled("plugin/SameTextModule_AbsolutePathAnsweredExternal", ({ root }) => { + const declined = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import x from "./x.js"; + import "./late.js"; + console.log(x); + `, + "/x.js": `export default "bundled-x";`, + "/late.js": ``, + }, + plugins(builder) { + builder.onResolve({ filter: /x\.js$/ }, args => { + if (args.importer.endsWith("late.js")) return { path: args.path, external: true }; + declined.resolve(); + }); + builder.onLoad({ filter: /late\.js$/ }, async () => { + await declined.promise; + return { contents: `import y from ${JSON.stringify(join(root, "x.js"))}; console.log(y);`, loader: "js" }; + }); + }, + onAfterBundle(api) { + const contents = api.readFile("/out.js"); + expect(contents).toContain(`"bundled-x"`); + expect(contents).toMatch(/^import \w+ from "[^"]*x\.js";$/m); + }, + }; + }); + + // An import that is already linked to the real package keeps that link. + // first.js bundles the real "foo-pkg". late.js imports it again, after a + // plugin module with the text "foo-pkg" exists. + itBundled("plugin/SameTextModule_SecondImporterOfRealPackage", () => { + const virtAnswered = Promise.withResolvers(); + const realLoaded = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import first from "./first.js"; + import { x } from "virt"; + import late from "./late.js"; + console.log(first, x, late); + `, + "/first.js": `import real from "foo-pkg"; export default real;`, + "/late.js": ``, + ...realFooPkg, + }, + plugins(builder) { + builder.onResolve({ filter: /^virt$/ }, () => { + virtAnswered.resolve(); + return { path: "foo-pkg", namespace: "virt" }; + }); + builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { + return { contents: `export const x = "x"; export default "virt";`, loader: "js" }; + }); + builder.onLoad({ filter: /foo-pkg[\\/]index\.js$/ }, () => { + realLoaded.resolve(); + return undefined; + }); + builder.onLoad({ filter: /late\.js$/ }, async () => { + await Promise.all([virtAnswered.promise, realLoaded.promise]); + return { contents: `import real from "foo-pkg"; export default real;`, loader: "js" }; + }); + }, + run: { stdout: "real x real" }, + }; + }); + + // redir.cjs is only `module.exports = require("foo-pkg")`, so the bundler + // links its importers straight to what that require() resolves to. Both + // importers must get the same thing, and never the plugin module with the + // text "foo-pkg". The second importer is loaded only after the require() is + // decided. + for (const { name, external, answer, stdout } of [ + { name: "ExternalOption", external: ["foo-pkg"], stdout: "true real real" }, + { + name: "PluginAnswersExternal", + answer: (args: { path: string }) => ({ path: args.path, external: true }), + stdout: "true real real", + }, + { + name: "PluginAnswersModule", + answer: (_: { path: string }, root: string) => ({ path: join(root, "chosen.cjs") }), + stdout: "true chosen chosen", + }, + ] as { + name: string; + external?: string[]; + answer?: (args: { path: string }, root: string) => object; + stdout: string; + }[]) { + itBundled(`plugin/SameTextModule_RedirectFile_${name}`, ({ root }) => { + const decided = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import wrapper from "wrapped-foo"; + import first from "gate:first"; + import second from "gate:second"; + console.log(wrapper.isWrapper, first, second); + `, + "/redir.cjs": `module.exports = require("foo-pkg");`, + "/chosen.cjs": `module.exports = "chosen";`, + ...realFooPkg, + }, + external, + plugins(builder) { + const importsRedir = `import r from ${JSON.stringify(join(root, "redir.cjs"))}; export default r;`; + builder.onResolve({ filter: /^wrapped-foo$/ }, () => { + return { path: "foo-pkg", namespace: "wrap" }; + }); + builder.onLoad({ filter: /.*/, namespace: "wrap" }, () => { + return { contents: `export default { isWrapper: true };`, loader: "js" }; + }); + builder.onResolve({ filter: /^gate:/ }, args => { + return { path: args.path.slice("gate:".length), namespace: "gate" }; + }); + builder.onLoad({ filter: /^first$/, namespace: "gate" }, () => { + return { contents: importsRedir, loader: "js" }; + }); + builder.onLoad({ filter: /^second$/, namespace: "gate" }, async () => { + await decided.promise; + return { contents: importsRedir, loader: "js" }; + }); + if (answer) { + builder.onResolve({ filter: /^foo-pkg$/ }, args => { + decided.resolve(); + return answer(args, root) as never; + }); + } else { + builder.onLoad({ filter: /redir\.cjs$/ }, () => { + decided.resolve(); + return undefined; + }); + } + }, + run: { stdout }, + }; + }); + } + + // A barrel's records are resolved again each time a consumer un-defers one of + // them. The plugin's external answer for "react" must survive that: the + // bundled "virt" module has the same text as the path the plugin returned. + // late.js is loaded only after both answers are queued, so its un-defer of + // `a` runs after the external answer landed. + itBundled("plugin/SameTextModule_ExternalAnswerSurvivesBarrelUnDefer", () => { + const reactAnswered = Promise.withResolvers(); + const virtAnswered = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import { React, x } from "barrel"; + import "./late.js"; + console.log(React, x); + `, + "/late.js": ``, + "/node_modules/barrel/package.json": JSON.stringify({ + name: "barrel", + main: "./index.js", + sideEffects: false, + }), + "/node_modules/barrel/index.js": /* js */ ` + export { default as React } from "react"; + export { x } from "virt"; + export { a } from "./a.js"; + export { b } from "./b.js"; + `, + "/node_modules/barrel/a.js": `export const a = "a";`, + "/node_modules/barrel/b.js": `export const b = "b";`, + }, + plugins(builder) { + builder.onResolve({ filter: /^react$/ }, () => { + reactAnswered.resolve(); + return { path: "react-vendored", external: true }; + }); + builder.onResolve({ filter: /^virt$/ }, () => { + virtAnswered.resolve(); + return { path: "react-vendored", namespace: "virt" }; + }); + builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { + return { contents: `export const x = "x";`, loader: "js" }; + }); + builder.onLoad({ filter: /late\.js$/ }, async () => { + await Promise.all([reactAnswered.promise, virtAnswered.promise]); + return { contents: `import { a } from "barrel"; console.log(a);`, loader: "js" }; + }); + }, + onAfterBundle(api) { + const contents = api.readFile("/out.js"); + expect(contents).toContain(`from "react-vendored"`); + expect(contents).toContain(`"x"`); + expect(contents).not.toContain(`"react"`); + }, + }; + }); + itBundled("plugin/ResolveOverrideFile", ({ root }) => { return { files: { From 7ef9a92e0511ec838d4550336ba20bd83a270859 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:50:48 +0000 Subject: [PATCH 02/10] ci: retrigger From cdd4b8ffc2c98753e35beb2198c39f5de0f766fb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:29:14 +0000 Subject: [PATCH 03/10] bundler: shorten the comments on the module path flag and its lookup Comment-only. Each new comment is one line, the doc of patch_import_record_source_indices is unchanged, and the HMR duplicate note in the barrel code keeps its text from main except for the last sentence. --- src/ast/import_record.rs | 9 ++------- src/bundler/PathToSourceIndexMap.rs | 4 +--- src/bundler/barrel_imports.rs | 15 ++++++--------- src/bundler/bundle_v2.rs | 3 +-- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/ast/import_record.rs b/src/ast/import_record.rs index a8d421104730..73cd8f785de7 100644 --- a/src/ast/import_record.rs +++ b/src/ast/import_record.rs @@ -108,18 +108,13 @@ bitflags::bitflags! { /// import), and those records must never be resolved. const IS_BARREL_DEFERRED = 1 << 18; - /// The bundler resolved this record to a module and stored that - /// module's path in `path` (`ImportRecord::set_module_path`). Without - /// this flag `path` can still be the specifier (an external, a builtin, - /// a failed or pending resolve), and a module can have the same text. - /// Only valid while the bundler scans: the linker repoints `path` later. + /// Scan phase: `path` is the resolved module's path, not a specifier. See `set_module_path`. const PATH_IS_MODULE = 1 << 19; } } impl ImportRecord { - /// Point `path` at the module this record resolved to. The bundler's path - /// map pass sets `source_index` from it. + /// Store the resolved module's path. The bundler's path map pass links only such records. #[inline] pub fn set_module_path(&mut self, path: Path<'static>) { self.path = path; diff --git a/src/bundler/PathToSourceIndexMap.rs b/src/bundler/PathToSourceIndexMap.rs index edb6fc381af1..1cacb52eb51c 100644 --- a/src/bundler/PathToSourceIndexMap.rs +++ b/src/bundler/PathToSourceIndexMap.rs @@ -18,9 +18,7 @@ pub type Map = StringHashMap; pub(crate) type GetOrPutResult<'a> = bun_collections::string_hash_map::GetOrPutResult<'a, IndexInt>; impl PathToSourceIndexMap { - /// The module `record` resolved to. `None` unless resolution stored that - /// module's path in `record.path`: any other path text is a specifier, and - /// a module with the same text is a different thing. + /// Without `PATH_IS_MODULE` the path is a specifier, and a module can have the same text. #[inline] pub(crate) fn get_record(&self, record: &ImportRecord) -> Option { if !record.flags.contains(ImportRecordFlags::PATH_IS_MODULE) { diff --git a/src/bundler/barrel_imports.rs b/src/bundler/barrel_imports.rs index 9a322a8c454d..88ffad4bd6e5 100644 --- a/src/bundler/barrel_imports.rs +++ b/src/bundler/barrel_imports.rs @@ -326,10 +326,7 @@ fn record_target( map?.get_record(rec) } -/// The record that holds the target of `ir`. ConvertESMExportsForHmr marks a -/// duplicate import of one specifier as unused, so it is never resolved: its -/// target is the one of the surviving record (`dedup_fallback`, keyed by -/// specifier). See #28886. +/// The surviving record of an HMR duplicate import (`dedup_fallback`), else `ir` itself. #[inline] fn surviving_record<'a>( records: &'a [bun_ast::ImportRecord], @@ -500,11 +497,11 @@ pub(crate) fn schedule_barrel_deferred_imports( // In HMR, ConvertESMExportsForHmr deduplicates import records by path: // two `import { X } from 'mod'` statements become one, and the second // record is marked is_unused=true. resolveImportRecords then skips those - // records, so they never get a target of their own. named_imports entries - // created for the dedup'd record still point at its index. Build a - // fallback: raw specifier → index of the surviving record, using the - // non-unused records in this file that resolved to a module. See - // `surviving_record`. + // records, so their path.text stays as the raw specifier while the + // 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 → index of the surviving record. See #28886. let mut dedup_fallback: StringArrayHashMap = StringArrayHashMap::default(); if dev_handle.is_some() { for (idx, ir_probe) in file_import_records.as_slice().iter().enumerate() { diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index fe538b4247ab..2a61deaad684 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -7406,8 +7406,7 @@ pub mod bv2_impl { impl<'a> BundleV2<'a> { /// Patch source_index on import records from pathToSourceIndexMap and /// resolve_tasks_waiting_for_import_source_index. Called after - /// processResolveQueue has registered new modules. The map is only asked - /// about records that `resolve_import_records` pointed at a module. + /// processResolveQueue has registered new modules. pub(crate) fn patch_import_record_source_indices( &mut self, import_records: &mut import_record::List, From 72d5130b804aacc292f850bb85f86e68d58a2399 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 21:49:43 +0000 Subject: [PATCH 04/10] dev server: look up an import record by its path only when it points at a module The dev server has two lookups of its own by the path of an import record: the edge attachment in IncrementalGraph and the import list of an HTML file. An unlinked record that does not have PATH_IS_MODULE holds a specifier. When a plugin marked an import external with the path of a file that another route bundled, the route got that file: its stylesheet was added to the page, and its script was put in the client bundle. Both lookups now skip such a record. A record with a source_index is looked up as before. The two dev server tests and the tsconfig "paths" test come from the branch robobun/98024376/link-import-records-from-resolve, which fixed the same bug another way and reported the dev server output. The tsconfig test needs no plugin and no `external`: the import is linked to the right module by resolution, and the path map pass replaced that link. --- src/runtime/bake/DevServer.rs | 7 ++ .../bake/dev_server/incremental_graph.rs | 16 +++- test/bake/dev/plugins.test.ts | 95 ++++++++++++++++++- test/bundler/bundler_edgecase.test.ts | 20 ++++ 4 files changed, 133 insertions(+), 5 deletions(-) diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index b3353e38ff64..e21c13bc3420 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -2838,6 +2838,13 @@ impl DevServer { continue; // ignore non-JavaScript imports } } else { + // Without `PATH_IS_MODULE` the path is a specifier, not an in-graph file. + if !import + .flags + .contains(bun_ast::ImportRecordFlags::PATH_IS_MODULE) + { + continue; + } // Find the in-graph import. let Some(file) = self.client_graph.bundled_files.get(import.path.text) else { continue; diff --git a/src/runtime/bake/dev_server/incremental_graph.rs b/src/runtime/bake/dev_server/incremental_graph.rs index 14494e363a47..4bf69a27d2a8 100644 --- a/src/runtime/bake/dev_server/incremental_graph.rs +++ b/src/runtime/bake/dev_server/incremental_graph.rs @@ -1011,10 +1011,18 @@ impl IncrementalGraph { // Invalid source indices in CSS are external URLs. EdgeAttachmentMode::Css => return Ok(EdgeAttachmentResult::Stop), // Check IncrementalGraph for a file from a prior build. - EdgeAttachmentMode::JsOrHtml => match self.bundled_files.get_index(key) { - Some(i) => (FileIndex::::init(i as u32), FileKind::Unknown), - None => return Ok(EdgeAttachmentResult::Continue), - }, + EdgeAttachmentMode::JsOrHtml => { + // Not linked and not pointed at a module: `key` is a specifier, not a file. + if !ir_source_index.is_valid() + && !ir_flags.contains(bun_ast::ImportRecordFlags::PATH_IS_MODULE) + { + return Ok(EdgeAttachmentResult::Continue); + } + match self.bundled_files.get_index(key) { + Some(i) => (FileIndex::::init(i as u32), FileKind::Unknown), + None => return Ok(EdgeAttachmentResult::Continue), + } + } } }; diff --git a/test/bake/dev/plugins.test.ts b/test/bake/dev/plugins.test.ts index fa1641f878af..bd52cc05eeb2 100644 --- a/test/bake/dev/plugins.test.ts +++ b/test/bake/dev/plugins.test.ts @@ -1,5 +1,6 @@ // Plugin tests concern plugins in development mode. -import { devTest, minimalFramework } from "../bake-harness"; +import { expect } from "bun:test"; +import { Dev, devTest, emptyHtmlFile, minimalFramework } from "../bake-harness"; // Note: more in depth testing of plugins is done in test/bundler/bundler_plugin.test.ts devTest("onResolve", { @@ -110,6 +111,98 @@ devTest("onResolve + onLoad virtual file", { ]); }, }); +// The plugin marks the import in external.ts external, with the path that styles.css is +// bundled under for styled.ts. The dev server must not take that path for its own file. +devTest("a plugin-external import whose path is a file another route bundled adds no stylesheet", { + files: { + "bunfig.toml": ` + [serve.static] + plugins = ["./external-styles-plugin.ts"] + `, + "external-styles-plugin.ts": ` + import * as path from 'path'; + export default { + name: 'external-styles', + setup(build) { + build.onResolve({ filter: /^external-styles$/ }, (args) => { + return { path: path.join(args.resolveDir, 'styles.css'), external: true }; + }); + }, + }; + `, + "styled.html": emptyHtmlFile({ scripts: ["styled.ts"] }), + "styled.ts": ` + import './styles.css'; + `, + "external.html": emptyHtmlFile({ scripts: ["external.ts"] }), + "external.ts": ` + import 'external-styles'; + `, + "styles.css": ` + .box { color: red; } + `, + }, + async test(dev) { + // A fetch returns after its route is bundled, so styles.css is a file of the dev server + // before the plugin answers the import in external.ts. + expect(await fetchPage(dev, "/styled")).toEqual({ + status: 200, + stylesheets: [expect.stringMatching(/\.css$/)], + }); + expect(await fetchPage(dev, "/external")).toEqual({ + status: 200, + stylesheets: [], + }); + }, +}); +// The same for a script: the plugin marks the script of external.html external, with the +// path that page.ts is bundled under for page.html. The bundle of external.html has no page.ts. +devTest("a plugin-external script whose path is a file another route bundled is not in the bundle", { + files: { + "bunfig.toml": ` + [serve.static] + plugins = ["./external-script-plugin.ts"] + `, + "external-script-plugin.ts": ` + import * as path from 'path'; + export default { + name: 'external-script', + setup(build) { + build.onResolve({ filter: /^external-script$/ }, (args) => { + return { path: path.join(args.resolveDir, 'page.ts'), external: true }; + }); + }, + }; + `, + "page.html": emptyHtmlFile({ scripts: ["page.ts"] }), + "page.ts": ` + console.log("PAGE_SCRIPT_RAN"); + `, + "external.html": emptyHtmlFile({ scripts: ["external-script"] }), + }, + async test(dev) { + expect(await fetchClientBundle(dev, "/page")).toContain("PAGE_SCRIPT_RAN"); + const bundle = await fetchClientBundle(dev, "/external"); + expect(bundle).not.toContain("PAGE_SCRIPT_RAN"); + expect(bundle).not.toContain("page.ts"); + }, +}); + +async function fetchPage(dev: Dev, url: string) { + const response = await dev.fetch(url); + const html = await response.text(); + return { + status: response.status, + stylesheets: [...html.matchAll(/]*href="([^"]+)"/g)].map(match => match[1]), + }; +} + +async function fetchClientBundle(dev: Dev, url: string) { + const html = await (await dev.fetch(url)).text(); + const src = html.match(/src="(\/_bun\/client\/[^"]+)"/)![1]; + return await (await dev.fetch(src)).text(); +} + // devTest("onLoad with watchFile", { // framework: minimalFramework, // pluginFile: ` diff --git a/test/bundler/bundler_edgecase.test.ts b/test/bundler/bundler_edgecase.test.ts index 7ba9ba960064..8316cf1ac3f2 100644 --- a/test/bundler/bundler_edgecase.test.ts +++ b/test/bundler/bundler_edgecase.test.ts @@ -1842,6 +1842,26 @@ describe("bundler", () => { }, })); } + // tsconfig "paths" maps the specifier in late.js, the absolute path of a.js, to b.js. entry.js + // imports a.js too, so that specifier text is also the path of a module in the bundle. The + // import in late.js is b.js all the same. + itBundled("edgecase/TsconfigPathsTargetIsNotReplacedByTheModuleAtTheSpecifierPath", ({ root }) => ({ + files: { + "/entry.js": /* js */ ` + import { who as b } from "./b.js"; + import { who as a } from "./a.js"; + import "./late.js"; + console.log("entry", a, b); + `, + "/a.js": `export const who = "A";`, + "/b.js": `export const who = "B";`, + "/late.js": `import { who } from ${JSON.stringify(join(root, "a.js"))}; console.log("late", who);`, + "/tsconfig.json": JSON.stringify({ + compilerOptions: { baseUrl: ".", paths: { [join(root, "a.js")]: ["./b.js"] } }, + }), + }, + run: { stdout: "late B\nentry A B" }, + })); itBundled("edgecase/IntegerUnderflow#12547", { files: { "/entry.js": ` From b6cb155b9527db162b4ad018767db31b642a3fcf Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:30:29 +0000 Subject: [PATCH 05/10] [autofix.ci] apply automated fixes --- src/runtime/bake/dev_server/render.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/runtime/bake/dev_server/render.rs b/src/runtime/bake/dev_server/render.rs index b65e8a6f956e..c5fc82bb5713 100644 --- a/src/runtime/bake/dev_server/render.rs +++ b/src/runtime/bake/dev_server/render.rs @@ -471,7 +471,10 @@ impl DevServer { } } else { // Without `PATH_IS_MODULE` the path is a specifier, not an in-graph file. - if !import.flags.contains(bun_ast::ImportRecordFlags::PATH_IS_MODULE) { + if !import + .flags + .contains(bun_ast::ImportRecordFlags::PATH_IS_MODULE) + { continue; } // Find the in-graph import. From e3801052dd56671d9107d6b986612441ffe04202 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:02:24 +0000 Subject: [PATCH 06/10] bundler: move PATH_IS_MODULE to bit 22 of ImportRecordFlags Open pull requests add other flags at bits 19, 20 and 21 of this set. Two flags with one value compile and then read as each other. --- src/ast/import_record.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ast/import_record.rs b/src/ast/import_record.rs index 73cd8f785de7..a8ed8f54da11 100644 --- a/src/ast/import_record.rs +++ b/src/ast/import_record.rs @@ -109,7 +109,7 @@ bitflags::bitflags! { const IS_BARREL_DEFERRED = 1 << 18; /// Scan phase: `path` is the resolved module's path, not a specifier. See `set_module_path`. - const PATH_IS_MODULE = 1 << 19; + const PATH_IS_MODULE = 1 << 22; } } From 7e681efdb80bd07e201c028a01512c5170ab9395 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:02:45 +0000 Subject: [PATCH 07/10] dev server: keep the module path of an import whose stylesheet failed finish_from_bake_dev_server removes the source_index of an import of a stylesheet that failed in this bundle. An import that was linked by index only kept the text that was written: the link tag of a page that is bundled together with its stylesheet, or an import that a plugin resolved. The edge attachment skips a record that has no source_index and no PATH_IS_MODULE, so the page lost its stylesheet. It answered 200 with no link tag while the stylesheet was broken, and had no link tag after the stylesheet was repaired. The unlink now stores the path of the stylesheet with set_module_path. The record still points at its module, and the edge is attached by that path. On main the lookup uses the written text. That finds the file only when the text is its absolute path, so href="./x.css" and a plugin-resolved import lose the stylesheet there too. The four new tests in test/bake/dev/css.test.ts cover both. --- src/bundler/bundle_v2.rs | 4 +++ test/bake/dev/css.test.ts | 75 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 8429bbc207cf..e8d4e87f0db7 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -6056,6 +6056,10 @@ pub mod bv2_impl { (*parts_col.add(record.source_index.get() as usize)).len() } == 0 { + // The path is the record's only link to the stylesheet from here on. + record.set_module_path( + sources[record.source_index.get() as usize].path, + ); record.source_index = Index::INVALID; // The importer isn't bundled again when the stylesheet is fixed, so its // output must treat this import the same as a stylesheet that didn't fail. diff --git a/test/bake/dev/css.test.ts b/test/bake/dev/css.test.ts index 9ce744ae8636..a64586cc7566 100644 --- a/test/bake/dev/css.test.ts +++ b/test/bake/dev/css.test.ts @@ -1,7 +1,7 @@ // CSS tests concern bundling bugs with CSS files import { expect } from "bun:test"; import assert from "node:assert"; -import { devTest, emptyHtmlFile, imageFixtures } from "../bake-harness"; +import { Dev, devTest, emptyHtmlFile, imageFixtures } from "../bake-harness"; devTest("css file with syntax error does not kill old styles", { files: { @@ -725,6 +725,51 @@ devTest("an error in a stylesheet reaches a page through an @import that another expect(await load()).toEqual(["3px", "3px"]); }, }); +// A page that is bundled together with its stylesheet has its link tag linked by index, so the +// import keeps the text that was written. When the stylesheet fails, that link is removed. +for (const href of ["main.css", "/main.css", "./main.css"]) { + devTest(`a page recovers after it was rebuilt together with its broken stylesheet (href="${href}")`, { + files: { + "index.html": emptyHtmlFile({ styles: [href] }), + "main.css": `.main { width: 1px; }`, + }, + async test(dev) { + await breakStylesheetWithItsImporter(dev, { + stylesheet: "main.css", + importer: "index.html", + editedImporter: emptyHtmlFile({ styles: [href], body: "

edited

" }), + }); + }, + }); +} +// The same for an import that a plugin resolved: it is linked by index and keeps its specifier. +devTest("a page recovers after its script was rebuilt together with a broken stylesheet that a plugin resolved", { + files: { + "bunfig.toml": ` + [serve.static] + plugins = ["./alias-plugin.ts"] + `, + "alias-plugin.ts": ` + import * as path from "path"; + export default { + name: "alias-plugin", + setup(build) { + build.onResolve({ filter: /^aliased-css$/ }, args => ({ path: path.join(args.resolveDir, "real.css") })); + }, + }; + `, + "index.html": emptyHtmlFile({ scripts: ["main.ts"] }), + "main.ts": `import "aliased-css";`, + "real.css": `.main { width: 1px; }`, + }, + async test(dev) { + await breakStylesheetWithItsImporter(dev, { + stylesheet: "real.css", + importer: "main.ts", + editedImporter: `import "aliased-css"; console.log("edited");`, + }); + }, +}); devTest("css import before create", { files: { "index.html": emptyHtmlFile({ @@ -811,6 +856,34 @@ devTest("css import before create project relative", { }, }); +/** + * One bundle has a syntax error in the stylesheet and an edit of the file that imports it. + * The page fails while the stylesheet is broken, and has the stylesheet again once it is fixed. + */ +async function breakStylesheetWithItsImporter( + dev: Dev, + { stylesheet, importer, editedImporter }: { stylesheet: string; importer: string; editedImporter: string }, +) { + // The status of the page, or the width of ".main" in its stylesheet. + const load = async () => { + const page = await dev.fetch("/"); + const html = await page.text(); + if (page.status !== 200) return page.status; + const asset = html.match(/\/_bun\/asset\/\w+\.css/)?.[0]; + if (!asset) return "no stylesheet"; + return (await dev.fetch(asset).text()).match(/\.main \{[^}]*width: (\w+)/)?.[1]; + }; + expect(await load()).toBe("1px"); + { + await using batch = await dev.batchChanges({ errors: null }); + await dev.write(stylesheet, `.main { width: 2px; }}}`); + await dev.write(importer, editedImporter); + } + expect(await load()).toBe(500); + await dev.write(stylesheet, `.main { width: 3px; }`, { errors: null }); + expect(await load()).toBe("3px"); +} + function extractCssUrl(backgroundImage: string): string { const url = backgroundImage.match(/url\((['"])(.*?)\1\)/); if (!url) { From 11c7c2ce4ee52ac6acb71dd056a90ce0ec3e4a61 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:40:06 +0000 Subject: [PATCH 08/10] test(bundler): un-defer the barrel record that a plugin answers external The old case un-deferred another record of the barrel. An un-defer resolves only the records it selects, so that case passed without the change in the path map lookup. --- test/bundler/bundler_plugin.test.ts | 45 +++++++++++------------------ 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index b779f9c6ccea..59aa8d6b8734 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -881,20 +881,19 @@ describe("bundler", () => { }); } - // A barrel's records are resolved again each time a consumer un-defers one of - // them. The plugin's external answer for "react" must survive that: the - // bundled "virt" module has the same text as the path the plugin returned. - // late.js is loaded only after both answers are queued, so its un-defer of - // `a` runs after the external answer landed. - itBundled("plugin/SameTextModule_ExternalAnswerSurvivesBarrelUnDefer", () => { - const reactAnswered = Promise.withResolvers(); + // A late importer un-defers a barrel re-export, and a plugin answers external + // for it. The bundler links the un-deferred record while that answer is + // pending, when the record still has its specifier "react", which is also the + // text of the "virt" module. late.js is loaded only after the "virt" answer is + // queued, so that module is in the path map by then. + itBundled("plugin/SameTextModule_BarrelUnDeferAnsweredExternal", () => { const virtAnswered = Promise.withResolvers(); return { files: { "/entry.js": /* js */ ` - import { React, x } from "barrel"; - import "./late.js"; - console.log(React, x); + import { x } from "barrel"; + import late from "./late.js"; + console.log(x, late); `, "/late.js": ``, "/node_modules/barrel/package.json": JSON.stringify({ @@ -905,35 +904,25 @@ describe("bundler", () => { "/node_modules/barrel/index.js": /* js */ ` export { default as React } from "react"; export { x } from "virt"; - export { a } from "./a.js"; - export { b } from "./b.js"; `, - "/node_modules/barrel/a.js": `export const a = "a";`, - "/node_modules/barrel/b.js": `export const b = "b";`, + "/node_modules/react/package.json": JSON.stringify({ name: "react", main: "./index.js" }), + "/node_modules/react/index.js": `module.exports = "real";`, }, plugins(builder) { - builder.onResolve({ filter: /^react$/ }, () => { - reactAnswered.resolve(); - return { path: "react-vendored", external: true }; - }); + builder.onResolve({ filter: /^react$/ }, args => ({ path: args.path, external: true })); builder.onResolve({ filter: /^virt$/ }, () => { virtAnswered.resolve(); - return { path: "react-vendored", namespace: "virt" }; + return { path: "react", namespace: "virt" }; }); builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { - return { contents: `export const x = "x";`, loader: "js" }; + return { contents: `export const x = "x"; export default "virt";`, loader: "js" }; }); builder.onLoad({ filter: /late\.js$/ }, async () => { - await Promise.all([reactAnswered.promise, virtAnswered.promise]); - return { contents: `import { a } from "barrel"; console.log(a);`, loader: "js" }; + await virtAnswered.promise; + return { contents: `import { React } from "barrel"; export default React;`, loader: "js" }; }); }, - onAfterBundle(api) { - const contents = api.readFile("/out.js"); - expect(contents).toContain(`from "react-vendored"`); - expect(contents).toContain(`"x"`); - expect(contents).not.toContain(`"react"`); - }, + run: { stdout: "x real" }, }; }); From a3f1f63c751278c92d039bd531d065fee70a1382 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:48:29 +0000 Subject: [PATCH 09/10] bundler: a test for each lookup by record and for the FileMap store Three lookups and one store had no test that fails without them: - The `export *` target lookup in on_parse_task_complete. barrel/ExternalExportStarIsNotATargetOfModuleWithSameText fails on main. - record_target in the dev server. "an external import with the path of the barrel requests nothing" in test/bake/dev/bundle.test.ts fails on main. - The dedup_fallback filter, which now takes a surviving record that is linked by index. "two import statements from a barrel that a plugin declined" fails on main. - The FileMap store for a file that is already queued. "in-memory file imported twice by one file" passes on main and fails when that store does not set the flag. Each of the four fails on a build with only its line turned back. --- test/bake/dev/bundle.test.ts | 93 +++++++++++++++++++++++++++++ test/bundler/bundler_barrel.test.ts | 48 +++++++++++++++ test/bundler/bundler_files.test.ts | 26 ++++++++ 3 files changed, 167 insertions(+) diff --git a/test/bake/dev/bundle.test.ts b/test/bake/dev/bundle.test.ts index 6e7e8359aa46..0b397d427ec2 100644 --- a/test/bake/dev/bundle.test.ts +++ b/test/bake/dev/bundle.test.ts @@ -1409,6 +1409,99 @@ devTest("barrel optimization: two import statements from the same barrel (#28886 }, }); +const alphaBetaGammaBarrel = { + "node_modules/barrel-lib/package.json": JSON.stringify({ + name: "barrel-lib", + version: "1.0.0", + main: "./index.js", + sideEffects: false, + }), + "node_modules/barrel-lib/index.js": ` + export { Alpha } from './alpha.js'; + export { Beta } from './beta.js'; + export { Gamma } from './gamma.js'; + `, + "node_modules/barrel-lib/alpha.js": `export const Alpha = () => "ALPHA";`, + "node_modules/barrel-lib/beta.js": `export const Beta = () => "BETA";`, + "node_modules/barrel-lib/gamma.js": `export const Gamma = () => "GAMMA";`, +}; +// Which submodules of the barrel are in the client bundle. +async function barrelSubmodulesInBundle(dev: Dev) { + const bundle = await clientBundle(dev); + return ["ALPHA", "BETA", "GAMMA"].filter(name => bundle.includes(`"${name}"`)); +} + +// The same as #28886 for an import that an onResolve plugin declined. The resolver +// then links the surviving record by index, and the record keeps its specifier. +devTest("barrel optimization: two import statements from a barrel that a plugin declined", { + files: { + "bunfig.toml": ` + [serve.static] + plugins = ["./decline-plugin.ts"] + `, + "decline-plugin.ts": ` + export default { + name: "decline", + setup(build) { + build.onResolve({ filter: /^barrel-lib$/ }, () => undefined); + }, + }; + `, + "index.html": emptyHtmlFile({ scripts: ["index.ts"] }), + "index.ts": ` + import { Alpha } from 'barrel-lib'; + import { Beta } from 'barrel-lib'; + console.log('got: ' + Alpha() + ' ' + Beta()); + `, + ...alphaBetaGammaBarrel, + }, + async test(dev) { + expect(await barrelSubmodulesInBundle(dev)).toEqual(["ALPHA", "BETA"]); + }, +}); + +// An import that a plugin answers external is not a module. Here its path is the path +// of the barrel, and it must not request Beta from the barrel through that text. +// "./late.ts" is answered after the external answer, so index.ts is scanned for its +// requests once more while the external record holds the barrel's path. +devTest("barrel optimization: an external import with the path of the barrel requests nothing", { + files: { + "bunfig.toml": ` + [serve.static] + plugins = ["./barrel-path-plugin.ts"] + `, + "barrel-path-plugin.ts": ` + import * as path from "path"; + export default { + name: "barrel-path", + setup(build) { + const externalAnswered = Promise.withResolvers(); + build.onResolve({ filter: /^external-beta$/ }, args => { + externalAnswered.resolve(); + return { path: path.join(args.resolveDir, "node_modules/barrel-lib/index.js"), external: true }; + }); + build.onResolve({ filter: /^\\.\\/late\\.ts$/ }, async args => { + await externalAnswered.promise; + return { path: path.join(args.resolveDir, "late.ts") }; + }); + }, + }; + `, + "index.html": emptyHtmlFile({ scripts: ["index.ts"] }), + "index.ts": ` + import { Alpha } from 'barrel-lib'; + import { Beta } from 'external-beta'; + import { late } from './late.ts'; + console.log('got: ' + Alpha() + ' ' + Beta() + ' ' + late); + `, + "late.ts": `export const late = "LATE";`, + ...alphaBetaGammaBarrel, + }, + async test(dev) { + expect(await barrelSubmodulesInBundle(dev)).toEqual(["ALPHA"]); + }, +}); + // HMR lowering merges the statements of one import path into the first one. The // import record of each later statement is never resolved. const abBarrel = `export { a } from './a.js'; export { b } from './b.js';`; diff --git a/test/bundler/bundler_barrel.test.ts b/test/bundler/bundler_barrel.test.ts index c5f31427c416..3c6db7e87aac 100644 --- a/test/bundler/bundler_barrel.test.ts +++ b/test/bundler/bundler_barrel.test.ts @@ -1675,6 +1675,54 @@ describe("bundler", () => { }; }); + // An `export *` that a plugin answers external has no module. Its specifier is + // the path of the barrel, and the barrel must not become an `export *` target + // through that text: a target defers nothing, so b.js would load. The barrel is + // loaded only after star.js is scanned, which is when a target is marked. + itBundled("barrel/ExternalExportStarIsNotATargetOfModuleWithSameText", ({ root }) => { + const starScanned = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import { A } from 'starlib'; + import './star.js'; + console.log(A); + `, + "/star.js": `export * from ${JSON.stringify(join(root, "node_modules/starlib/index.js"))};`, + "/node_modules/starlib/package.json": JSON.stringify({ + name: "starlib", + main: "./index.js", + sideEffects: false, + }), + "/node_modules/starlib/index.js": /* js */ ` + export { A } from './a.js'; + export { B } from './b.js'; + `, + "/node_modules/starlib/a.js": /* js */ ` + export const A = "star-lib-a"; + `, + // Nobody imports B, so b.js must stay deferred. + "/node_modules/starlib/b.js": /* js */ ` + export const B = <<>>; + `, + }, + outfile: "/out.js", + plugins(builder) { + builder.onResolve({ filter: /starlib[\\/]index\.js$/ }, args => { + starScanned.resolve(); + return { path: args.path, external: true }; + }); + builder.onLoad({ filter: /starlib[\\/]index\.js$/ }, async () => { + await starScanned.promise; + return undefined; + }); + }, + onAfterBundle(api) { + api.expectFile("/out.js").toContain("star-lib-a"); + }, + }; + }); + // When the plugin does not answer, the record falls back to the resolver and // the failure is reported. Resolving the barrel again reported it twice. itBundled("barrel/UnresolvableReExportReportedOnce", { diff --git a/test/bundler/bundler_files.test.ts b/test/bundler/bundler_files.test.ts index 9e19987bdf3c..f50ca556efea 100644 --- a/test/bundler/bundler_files.test.ts +++ b/test/bundler/bundler_files.test.ts @@ -133,6 +133,32 @@ describe("bundler files option", () => { expect(output).toContain("B:"); }); + test("in-memory file imported twice by one file", async () => { + const result = await Bun.build({ + entrypoints: ["/entry.js"], + files: { + "/entry.js": ` + import lib from "./lib.js"; + import { named } from "./lib.js"; + console.log(lib, named); + `, + "/lib.js": ` + export default "default-value"; + export const named = "named-value"; + `, + }, + }); + + expect(result.success).toBe(true); + expect(result.outputs.length).toBe(1); + + const output = await result.outputs[0].text(); + expect(output).toContain("default-value"); + expect(output).toContain("named-value"); + // The second statement is bundled too. It is not left as an import of a path that exists only in memory. + expect(output).not.toMatch(/^\s*import\b/m); + }); + test("in-memory file with nested imports", async () => { const result = await Bun.build({ entrypoints: ["/entry.js"], From e14cd97a8f6e27dcf8e2fa42fceb9e9a76f6d338 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:22:56 +0000 Subject: [PATCH 10/10] dev server: store the failure key of a failed stylesheet at its unlink finish_from_bake_dev_server stored the source path of a failed stylesheet in the import record. For a stylesheet of a plugin namespace, the graph key of that path is its pretty form, and the dev server files the failure under path.text. The edge from the importer missed the failure: a page with a syntax error in such a stylesheet answered 500, then 200. The record now gets a path of that text. The new case in test/bake/dev/css.test.ts has an html link, a script import, and an import whose specifier is not the path that the plugin returned. On main the first two pass and the third fails. --- src/bundler/bundle_v2.rs | 8 ++++---- test/bake/dev/css.test.ts | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index c8841362ceed..351eb6eafe56 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -5928,10 +5928,10 @@ pub mod bv2_impl { (*parts_col.add(record.source_index.get() as usize)).len() } == 0 { - // The path is the record's only link to the stylesheet from here on. - record.set_module_path( - sources[record.source_index.get() as usize].path, - ); + // The dev server files this failure under `path.text` in every namespace. + record.set_module_path(Fs::Path::init( + sources[record.source_index.get() as usize].path.text, + )); record.source_index = Index::INVALID; // The importer isn't bundled again when the stylesheet is fixed, so its // output must treat this import the same as a stylesheet that didn't fail. diff --git a/test/bake/dev/css.test.ts b/test/bake/dev/css.test.ts index a64586cc7566..45d2d2a72cf1 100644 --- a/test/bake/dev/css.test.ts +++ b/test/bake/dev/css.test.ts @@ -770,6 +770,43 @@ devTest("a page recovers after its script was rebuilt together with a broken sty }); }, }); +// A stylesheet that a plugin loads from its own namespace has a syntax error. The page keeps +// that build error on every load, also when the specifier is not the path the plugin returned. +devTest("a page keeps the build error of a stylesheet that a plugin loads from a namespace", { + files: { + "bunfig.toml": ` + [serve.static] + plugins = ["./virtual-modules.ts"] + `, + "virtual-modules.ts": ` + export default { + name: "virtual-modules", + setup(build) { + build.onResolve({ filter: /^virtual\\./ }, args => ({ path: args.path, namespace: "virtual" })); + build.onResolve({ filter: /^renamed\\./ }, args => ({ path: "virtual." + args.path, namespace: "virtual" })); + build.onLoad({ filter: /.*/, namespace: "virtual" }, () => ({ contents: "h1 { color: red; }}", loader: "css" })); + }, + }; + `, + "linked.html": emptyHtmlFile({ styles: ["virtual.0.css"] }), + "imported.html": emptyHtmlFile({ scripts: ["./entry.ts"] }), + "entry.ts": `import "virtual.1.css";`, + "renamed.html": emptyHtmlFile({ scripts: ["./renamed.ts"] }), + "renamed.ts": `import "renamed.2.css";`, + }, + async test(dev) { + const twoLoads = async (url: string) => [(await dev.fetch(url)).status, (await dev.fetch(url)).status]; + expect({ + linked: await twoLoads("/linked"), + imported: await twoLoads("/imported"), + renamed: await twoLoads("/renamed"), + }).toEqual({ + linked: [500, 500], + imported: [500, 500], + renamed: [500, 500], + }); + }, +}); devTest("css import before create", { files: { "index.html": emptyHtmlFile({