diff --git a/src/ast/import_record.rs b/src/ast/import_record.rs index fe565b61764b..30fcf983e79e 100644 --- a/src/ast/import_record.rs +++ b/src/ast/import_record.rs @@ -103,6 +103,18 @@ 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; + + /// Scan phase: `path` is the resolved module's path, not a specifier. See `set_module_path`. + const PATH_IS_MODULE = 1 << 22; + } +} + +impl<'a> ImportRecord<'a> { + /// 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<'a>) { + self.path = path; + self.flags.insert(Flags::PATH_IS_MODULE); } } diff --git a/src/bundler/PathToSourceIndexMap.rs b/src/bundler/PathToSourceIndexMap.rs index f0e42df43810..1cacb52eb51c 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,13 @@ 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()) + /// 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) { + 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 ae8bb4b5be1f..c7b04165d666 100644 --- a/src/bundler/barrel_imports.rs +++ b/src/bundler/barrel_imports.rs @@ -323,7 +323,22 @@ 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 surviving record of an HMR duplicate import (`dedup_fallback`), else `ir` itself. +#[inline] +fn surviving_record<'a>( + records: &'a [bun_ast::ImportRecord<'a>], + dedup_fallback: &StringArrayHashMap, + ir: &'a bun_ast::ImportRecord<'a>, +) -> &'a bun_ast::ImportRecord<'a> { + 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 @@ -486,11 +501,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<&[u8]> = StringArrayHashMap::default(); + // 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 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 +513,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 +540,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 +559,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 +619,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 706e739ccb98..351eb6eafe56 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -5928,6 +5928,10 @@ pub mod bv2_impl { (*parts_col.add(record.source_index.get() as usize)).len() } == 0 { + // 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. @@ -6790,13 +6794,14 @@ 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 = unsafe { &**resolve_entry.value_ptr }.path; + import_record + .set_module_path(unsafe { &**resolve_entry.value_ptr }.path); continue; } // For virtual files, use the path text as-is (no relative path computation needed). path_primary.pretty = self.graph.heap.alloc_slice_copy(path_primary.text); - import_record.path = path_primary; + import_record.set_module_path(path_primary); let _ = path_primary.text; // key already interned by get_or_put bun_core::scoped_log!( Bundle, @@ -7099,9 +7104,7 @@ pub mod bv2_impl { { // Overload `path.text` to point to the final URL // This information cannot be queried while printing because a lock wouldn't get held. - import_record.path.text = path.text; - import_record.path.namespace = b"file"; - import_record.path.pretty = self + let pretty = self .arena() .alloc_str(&format!( "{}/{}{}", @@ -7112,13 +7115,20 @@ pub mod bv2_impl { bstr::BStr::new(bun_paths::extension(path.text)), )) .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 = self - .path_with_pretty_initialized(path, target) - .expect("oom"); + import_record.set_module_path( + self.path_with_pretty_initialized(path, target) + .expect("oom"), + ); if loader == Loader::Html || entry.kind == bake_types::CacheKind::Css { @@ -7159,8 +7169,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); } @@ -7174,7 +7185,7 @@ 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 = unsafe { &**resolve_entry.value_ptr }.path; + import_record.set_module_path(unsafe { &**resolve_entry.value_ptr }.path); continue; } @@ -7182,7 +7193,7 @@ pub mod bv2_impl { .path_with_pretty_initialized(path, target) .expect("oom"); - import_record.path = *path; + import_record.set_module_path(*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. @@ -7420,7 +7431,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() { @@ -7763,7 +7774,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/src/runtime/bake/dev_server/incremental_graph.rs b/src/runtime/bake/dev_server/incremental_graph.rs index 37b3b7e0f176..99ba08ac6b97 100644 --- a/src/runtime/bake/dev_server/incremental_graph.rs +++ b/src/runtime/bake/dev_server/incremental_graph.rs @@ -1034,10 +1034,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), - 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), + None => return Ok(EdgeAttachmentResult::Continue), + } + } } }; diff --git a/src/runtime/bake/dev_server/render.rs b/src/runtime/bake/dev_server/render.rs index 858e8f3a4726..344548a443cb 100644 --- a/src/runtime/bake/dev_server/render.rs +++ b/src/runtime/bake/dev_server/render.rs @@ -385,6 +385,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/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/bake/dev/css.test.ts b/test/bake/dev/css.test.ts index 9ce744ae8636..45d2d2a72cf1 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,88 @@ 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");`, + }); + }, +}); +// 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({ @@ -811,6 +893,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) { 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_barrel.test.ts b/test/bundler/bundler_barrel.test.ts index 5cb287c7b1b2..3c6db7e87aac 100644 --- a/test/bundler/bundler_barrel.test.ts +++ b/test/bundler/bundler_barrel.test.ts @@ -1626,6 +1626,103 @@ 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" }, + }; + }); + + // 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_edgecase.test.ts b/test/bundler/bundler_edgecase.test.ts index b65ae9be6569..b27d93a12849 100644 --- a/test/bundler/bundler_edgecase.test.ts +++ b/test/bundler/bundler_edgecase.test.ts @@ -1870,6 +1870,45 @@ 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"); + }, + })); + } + // 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" }, + })); // `bun build --compile --aot` resolves entry points with a resolver of its own, to the same answers. const itCompiledAheadOfTime = (id: string, opts: BundlerTestInput) => (unsupported ? itBundled.skip : itBundled)(`edgecase/aot/${id}`, { 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"], diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index 539dd4d2eb1a..59aa8d6b8734 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -608,6 +608,324 @@ 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 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 { x } from "barrel"; + import late from "./late.js"; + console.log(x, late); + `, + "/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"; + `, + "/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$/ }, args => ({ path: args.path, external: true })); + builder.onResolve({ filter: /^virt$/ }, () => { + virtAnswered.resolve(); + return { path: "react", namespace: "virt" }; + }); + builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { + return { contents: `export const x = "x"; export default "virt";`, loader: "js" }; + }); + builder.onLoad({ filter: /late\.js$/ }, async () => { + await virtAnswered.promise; + return { contents: `import { React } from "barrel"; export default React;`, loader: "js" }; + }); + }, + run: { stdout: "x real" }, + }; + }); + itBundled("plugin/ResolveOverrideFile", ({ root }) => { return { files: {