diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 201b32587eeb..f9fd691e618f 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -26,8 +26,8 @@ pub use bv2_impl::{DevServerInput, DevServerOutput, ImportTrackerIterator, Impor // `bundle_v2::Foo` rather than naming the implementation submodule. use self::bake_types as bake; pub use bv2_impl::{ - BuildResult, BundleV2Result, CompletionStruct, DependenciesScanner, DependenciesScannerResult, - OnDependenciesAnalyze, singleton, + BuildResult, BundleV2Result, CompletionStruct, DependenciesScanner, DependenciesScannerReads, + DependenciesScannerResult, OnDependenciesAnalyze, singleton, }; pub use crate::DeferredBatchTask::DeferredBatchTask; @@ -4328,6 +4328,16 @@ pub mod bv2_impl { pub entry_points: Box<[Box<[u8]>]>, pub(crate) on_fetch: fn(ctx: *mut (), result: &mut DependenciesScannerResult) -> Result<(), Error>, + pub(crate) reads: DependenciesScannerReads, + } + + /// What the callback of a [`DependenciesScanner`] reads. + #[derive(Clone, Copy, PartialEq, Eq)] + pub enum DependenciesScannerReads { + /// The import records of the files that the entry points reach. Nothing is linked. + ImportRecords, + /// The resolved exports of the entry point too. The bundle is linked first. + LinkedExports, } pub struct DependenciesScannerResult<'r, 'a> { @@ -4340,6 +4350,8 @@ pub mod bv2_impl { /// `Analyzer` struct implements this; [`DependenciesScanner::new`] erases the /// concrete type behind a monomorphized trampoline. pub trait OnDependenciesAnalyze { + const READS: DependenciesScannerReads; + fn on_analyze( &mut self, result: &mut DependenciesScannerResult<'_, '_>, @@ -4368,16 +4380,18 @@ pub mod bv2_impl { ctx: core::ptr::from_mut(analyzer).cast::<()>(), entry_points, on_fetch: trampoline::, + reads: A::READS, } } } impl<'a> BundleV2<'a> { + /// Hands the packages that `reachable_files` import to the scanner. A scan has no output files. pub(crate) fn get_all_dependencies( &mut self, reachable_files: &[Index], fetcher: &DependenciesScanner, - ) -> Result<(), Error> { + ) -> Result { // Find all external dependencies from reachable files let mut external_deps = bun_collections::StringSet::new(); @@ -4434,7 +4448,12 @@ pub mod bv2_impl { bundle_v2: self, reachable_files, }; - (fetcher.on_fetch)(fetcher.ctx, &mut result) + (fetcher.on_fetch)(fetcher.ctx, &mut result)?; + Ok(BuildResult { + output_files: Vec::new(), + metafile: None, + metafile_markdown: None, + }) } pub fn generate_from_cli( @@ -4493,6 +4512,12 @@ pub mod bv2_impl { let reachable_files = this.find_reachable_files()?; *reachable_files_count = reachable_files.len().saturating_sub(1); // - 1 for the runtime + if let Some(fetch) = fetcher + && fetch.reads == DependenciesScannerReads::ImportRecords + { + return this.get_all_dependencies(&reachable_files, fetch); + } + this.process_files_to_copy(&reachable_files)?; this.add_server_component_boundaries_as_extra_entry_points()?; @@ -4525,12 +4550,7 @@ pub mod bv2_impl { // Do this at the very end, after processing all the imports/exports so that we can follow exports as needed. if let Some(fetch) = fetcher { - this.get_all_dependencies(&reachable_files, fetch)?; - return Ok(BuildResult { - output_files: Vec::new(), - metafile: None, - metafile_markdown: None, - }); + return this.get_all_dependencies(&reachable_files, fetch); } let output_files = crate::linker_context_mod::generate_chunks_in_parallel::( diff --git a/src/runtime/cli/create_command.rs b/src/runtime/cli/create_command.rs index 9d53acf0954b..960e43e70a7f 100644 --- a/src/runtime/cli/create_command.rs +++ b/src/runtime/cli/create_command.rs @@ -1513,6 +1513,10 @@ struct Analyzer<'a> { } impl bun_bundler::bundle_v2::OnDependenciesAnalyze for Analyzer<'_> { + // `find_react_component_export` reads the resolved exports of the entry point. + const READS: bun_bundler::bundle_v2::DependenciesScannerReads = + bun_bundler::bundle_v2::DependenciesScannerReads::LinkedExports; + fn on_analyze( &mut self, result: &mut bun_bundler::bundle_v2::DependenciesScannerResult<'_, '_>, diff --git a/src/runtime/cli/pm_update_package_json.rs b/src/runtime/cli/pm_update_package_json.rs index a7502f1e1283..eef92b5d0308 100644 --- a/src/runtime/cli/pm_update_package_json.rs +++ b/src/runtime/cli/pm_update_package_json.rs @@ -1,7 +1,9 @@ //! `--analyze` runs the bundler, which `bun_install` cannot depend on, so that branch lives here. use crate::Error; -use bun_bundler::bundle_v2::{DependenciesScanner, DependenciesScannerResult}; +use bun_bundler::bundle_v2::{ + DependenciesScanner, DependenciesScannerReads, DependenciesScannerResult, +}; use bun_core::Global; use bun_install::package_manager_real::command_line_arguments::CommandLineArguments; use bun_install::package_manager_real::{Subcommand, update_package_json_and_install_and_cli}; @@ -45,6 +47,8 @@ pub(crate) fn analyze_dependencies_and_install( install: &'a mut dyn FnMut(&mut ContextData, &CommandLineArguments) -> Result<(), Error>, } impl bun_bundler::bundle_v2::OnDependenciesAnalyze for Analyzer<'_> { + const READS: DependenciesScannerReads = DependenciesScannerReads::ImportRecords; + fn on_analyze( &mut self, result: &mut DependenciesScannerResult<'_, '_>, diff --git a/test/cli/install/bun-add.test.ts b/test/cli/install/bun-add.test.ts index 0168a56d61f2..325634ed6e34 100644 --- a/test/cli/install/bun-add.test.ts +++ b/test/cli/install/bun-add.test.ts @@ -17,7 +17,7 @@ import { } from "harness"; import { createServer } from "http"; import type { AddressInfo } from "net"; -import { basename, join, relative, resolve } from "path"; +import { basename, dirname, join, relative, resolve } from "path"; import { pathToFileURL } from "url"; import { check_npm_auth_type, @@ -294,19 +294,38 @@ it("bun add --only-missing should not install existing package", async () => { } }); -// Writes `source` to entry-point.ts and returns what `bun --analyze` adds to package.json. `urls` collects -// its registry requests. +// Writes `source` to entry-point.ts and returns what `bun --analyze ` adds to package.json. `urls` +// collects its registry requests. `files` are written beside it. The registry answers 404 for the `absent` names. async function analyze( command: "add" | "install", source: string, - { packageJson = { name: "foo" }, urls = [] }: { packageJson?: object; urls?: string[] } = {}, + { + packageJson = { name: "foo" }, + urls = [], + files = {}, + absent = [], + entry = "./entry-point.ts", + }: { + packageJson?: object; + urls?: string[]; + files?: Record; + absent?: string[]; + entry?: string; + } = {}, ) { - setHandler(dummyRegistry(urls)); + const registry = dummyRegistry(urls); + setHandler(request => + absent.some(name => request.url.endsWith(`/${name}`)) ? new Response(null, { status: 404 }) : registry(request), + ); await writeFile(join(package_dir, "package.json"), JSON.stringify({ version: "0.0.1", ...packageJson })); await writeFile(join(package_dir, "entry-point.ts"), source); + for (const [path, contents] of Object.entries(files)) { + await mkdir(dirname(join(package_dir, path)), { recursive: true }); + await writeFile(join(package_dir, path), contents); + } // The registry has no tarball for most of these names. await using proc = spawn({ - cmd: [bunExe(), command, "--analyze", "--lockfile-only", "./entry-point.ts"], + cmd: [bunExe(), command, "--analyze", "--lockfile-only", entry], cwd: package_dir, stdout: "pipe", stderr: "pipe", @@ -416,6 +435,200 @@ it("bun add --analyze uses the nearest package.json with a name above the import expect(urls).toEqual([`${root_url}/bar`]); }); +// `--analyze` reads the import records and does not link the files as a browser bundle. Each of +// these is an error of that link. bun runs the first five. It rejects the last three too: two when it +// loads the file, one when it bundles the page. +it.each([ + [ + "a default import of an HTML page", + `import page from "./page.html"; import "bar"; console.log(page);`, + { + "page.html": ``, + "front.ts": `import "from-the-page-script";`, + }, + ["bar", "from-the-page-script"], + ], + [ + "a re-export of the default export of an HTML page", + `export { default as page } from "./page.html"; import "bar";`, + { "page.html": `

hi

` }, + ["bar"], + ], + [ + "require() of a file with top-level await", + `import "bar"; export const lazy = () => require("./tla.ts");`, + { "tla.ts": `export const value = await Promise.resolve(1);` }, + ["bar"], + ], + [ + "CSS composes of a class that the other file does not have", + `import styles from "./a.module.css"; import "bar"; console.log(styles);`, + { + "a.module.css": `.button { composes: missing from "./b.module.css"; }`, + "b.module.css": `.other { color: blue; }`, + }, + ["bar"], + ], + [ + "a CSS @import of a .ts file", + `import "./styles.css"; import "bar";`, + { "styles.css": `@import "./a.ts";`, "a.ts": `export const a = 1;` }, + ["bar"], + ], + [ + "an import of a name that the other file does not export", + `import { missing } from "./a.js"; import "bar"; console.log(missing);`, + { "a.js": `export const a = 1;` }, + ["bar"], + ], + [ + "an import of a name that two export * statements both export", + `import { twice } from "./star.js"; import "bar"; console.log(twice);`, + { + "star.js": `export * from "./a.js"; export * from "./b.js";`, + "a.js": `export const twice = 1;`, + "b.js": `export const twice = 2;`, + }, + ["bar"], + ], + [ + "an import of a missing name in the script of an HTML page", + `import page from "./page.html"; import "bar"; console.log(page);`, + { + "page.html": ``, + "front.js": `import { missing } from "./a.js"; console.log(missing);`, + "a.js": `export const a = 1;`, + }, + ["bar"], + ], +])("bun add --analyze installs across %s", async (_, source, files, added) => { + expect(await analyze("add", source, { files })).toEqual({ added, errors: [], exitCode: 0 }); +}); + +// Open: the scan is a browser bundle with every package external, so it does not read these two +// programs as bun runs them. The link used to stop both with an error. Each case fails until the +// scan is fixed. +// Now it adds `for-the-browser`: the scan follows the `browser` field, and bun loads impl.ts. +it.failing("bun add --analyze adds the packages of the file that bun loads, not of its browser twin", async () => { + expect( + await analyze("add", `import { server } from "./impl.ts"; console.log(server);`, { + packageJson: { name: "foo", browser: { "./impl.ts": "./impl.browser.ts" } }, + files: { + "impl.ts": `import "for-the-server"; export const server = 1;`, + "impl.browser.ts": `import "for-the-browser"; export const browser = 1;`, + }, + }), + ).toEqual({ added: ["for-the-server"], errors: [], exitCode: 0 }); +}); + +// Now it adds `bar` and `images`: the scan reads `images/photo.webp` as a file of a package. +it.failing("bun add --analyze reads a URL without ./ in an HTML page as a file beside the page", async () => { + expect( + await analyze("add", `import page from "./page.html"; import "bar"; console.log(page);`, { + files: { "page.html": ``, "images/photo.webp": "" }, + }), + ).toEqual({ added: ["bar"], errors: [], exitCode: 0 }); +}); + +it.each([ + ["the entry point has a syntax error", `import "bar"; export const value = ;`, {}, "error: Unexpected ;"], + [ + "a file that it imports has a syntax error", + `import "bar"; import "./broken.ts";`, + { "broken.ts": `export const value = ;` }, + "error: Unexpected ;", + ], +])("bun add --analyze adds nothing when %s", async (_, source, files, error) => { + const urls: string[] = []; + expect(await analyze("add", source, { files, urls })).toEqual({ added: [], errors: [error], exitCode: 1 }); + expect(urls).toEqual([]); +}); + +it("bun add --analyze adds nothing when the entry point does not exist", async () => { + const urls: string[] = []; + expect(await analyze("add", `import "bar";`, { entry: "./missing.ts", urls })).toEqual({ + added: [], + errors: [`error: ModuleNotFound resolving "./missing.ts" (entry point)`], + exitCode: 1, + }); + expect(urls).toEqual([]); +}); + +it.each(["add", "install"] as const)( + "bun %s --analyze adds nothing when the registry does not have a package", + async command => { + const { added, errors, exitCode } = await analyze(command, `import "bar"; import "absent";`, { + absent: ["absent"], + }); + expect({ added, errors: errors.slice(0, 1), exitCode }).toEqual({ + added: [], + errors: [`error: GET ${root_url}/absent - 404`], + exitCode: 1, + }); + }, +); + +// `bun create ` runs the same scan and reads the exports of the component, so it links. +// Its project bundles the component, and that bundle skips a re-export of a "sideEffects": false +// barrel that nothing imports. The scan has to skip it too. +describe("bun create and a file behind an unused re-export of a barrel", () => { + const files = { + "App.tsx": `import { a } from "./lib"; export default function App() { return
{a}
; }`, + "lib/index.ts": `export { a } from "./a"; export { Fancy } from "./fancy"; export { routes } from "./server";`, + "lib/a.ts": `export const a = 1;`, + "lib/fancy.tsx": `import "behind"; export const Fancy = () =>
;`, + "lib/server.ts": `import page from "./page.html"; export const routes = { "/": page };`, + "lib/page.html": `

hi

`, + }; + // The registry has no packages, so a run that reaches the install step fails there. + async function create(packageJson: object) { + using dir = tempDir("create-barrel", { ...files, "package.json": JSON.stringify(packageJson) }); + using registry = Bun.serve({ port: 0, fetch: () => new Response(null, { status: 404 }) }); + await using proc = spawn({ + cmd: [bunExe(), "create", "./App.tsx"], + cwd: String(dir), + env: { + ...env, + BUN_CONFIG_REGISTRY: registry.url.href, + // `bun create` exits without freeing the bundler. The ASAN lane would turn that into exit code 134. + ASAN_OPTIONS: [env.ASAN_OPTIONS, "detect_leaks=0"].filter(Boolean).join(":"), + }, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { + install: stdout.split(/\r?\n/).find(line => line.includes("--only-missing install")), + errors: stderr + .split(/\r?\n/) + .filter(line => line.startsWith("error: No matching export")) + .map(line => line.replaceAll("\\", "/")), + exitCode, + }; + } + + // Windows: `bun create` never prints the "--only-missing install" line this asserts on, so the + // dependency detection cannot be observed there. + it.todoIf(isWindows)("picks the template and the packages from the files that the bundle loads", async () => { + expect(await create({ name: "foo", sideEffects: false })).toEqual({ + install: "$ bun --only-missing install -- react-dom@19 react@19", + errors: [], + exitCode: 1, + }); + }); + + // Without the field the bundle loads lib/server.ts, and a browser bundle has no default export + // for an HTML page: `bun build ./App.tsx` stops with this error too. + it("stops where the browser bundle of the component stops", async () => { + expect(await create({ name: "foo" })).toEqual({ + install: undefined, + errors: [`error: No matching export in "lib/page.html" for import "default"`], + exitCode: 1, + }); + }); +}); + it("bun add --analyze should scan dependencies", async () => { const urls: string[] = []; setHandler(dummyRegistry(urls));