From 955c5615fb74327a70b642f07092a0b44977aa93 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:34:26 +0000 Subject: [PATCH 01/10] resolver: match sideEffects patterns without '/' at any depth Webpack (which defines the package.json sideEffects field) documents that array entries without a '/' are matched against the basename at any depth, equivalent to prefixing the pattern with '**/'. esbuild follows the same rule. Bun was joining such patterns directly onto the package directory, so '*.side.js' only matched files at the package root and a nested file the package declared as side-effectful was silently tree-shaken out of the bundle. When a sideEffects array entry contains no '/', store it as a glob with a '**/' prefix so it matches at any depth. Entries with an explicit path component (including a leading './') keep their existing root-relative behavior. --- src/resolver/package_json.rs | 31 ++++++++--- test/bundler/esbuild/dce.test.ts | 92 ++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index bbde9b52bcb9..a50b9223da71 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -703,10 +703,14 @@ impl PackageJSON { let mut has_globs = false; let mut has_exact = false; - // First pass: check if we have glob patterns and exact patterns + // First pass: check if we have glob patterns and exact patterns. + // Per webpack's documented semantics for this field, patterns + // without a '/' match against the basename at any depth (as if + // prefixed with "**/"), so they are always glob patterns. for item in items { if let Some(name) = item.as_str() { - if strings::contains_char(name, b'*') + if !strings::contains_char(name, b'/') + || strings::contains_char(name, b'*') || strings::contains_char(name, b'?') || strings::contains_char(name, b'[') || strings::contains_char(name, b'{') @@ -733,13 +737,19 @@ impl PackageJSON { continue; } + let slashless = !strings::contains_char(name, b'/'); + // Store the pattern relative to the package directory - let joined: [&[u8]; 2] = - [json_source.path.name().dir_with_trailing_slash(), name]; + let joined: [&[u8]; 3] = [ + json_source.path.name().dir_with_trailing_slash(), + if slashless { b"**/" } else { b"" }, + name, + ]; let pattern = r_fs.join(&joined); - if strings::contains_char(name, b'*') + if slashless + || strings::contains_char(name, b'*') || strings::contains_char(name, b'?') || strings::contains_char(name, b'[') || strings::contains_char(name, b'{') @@ -768,8 +778,15 @@ impl PackageJSON { } // Store the pattern relative to the package directory - let joined: [&[u8]; 2] = - [json_source.path.name().dir_with_trailing_slash(), name]; + let joined: [&[u8]; 3] = [ + json_source.path.name().dir_with_trailing_slash(), + if strings::contains_char(name, b'/') { + b"" + } else { + b"**/" + }, + name, + ]; let pattern = r_fs.join(&joined); // Normalize pattern to use forward slashes for cross-platform compatibility diff --git a/test/bundler/esbuild/dce.test.ts b/test/bundler/esbuild/dce.test.ts index 0e67d3a550b1..a5ae6e0ec78e 100644 --- a/test/bundler/esbuild/dce.test.ts +++ b/test/bundler/esbuild/dce.test.ts @@ -806,6 +806,98 @@ describe("bundler", () => { stdout: "main effect - should be preserved\ndone", }, }); + // Webpack (which defines this field) documents that patterns without a '/' + // match at any depth, like "**/pattern". esbuild follows the same rule. + itBundled("dce/PackageJsonSideEffectsGlobNoSlashMatchesNested", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/root.side.js"; + import "demo-pkg/deep/nested.side.js"; + import "demo-pkg/deep/also/here.side.js"; + import "demo-pkg/deep/unrelated.js"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/root.side.js": `console.log("root kept")`, + "/Users/user/project/node_modules/demo-pkg/deep/nested.side.js": `console.log("nested kept")`, + "/Users/user/project/node_modules/demo-pkg/deep/also/here.side.js": `console.log("deeper kept")`, + "/Users/user/project/node_modules/demo-pkg/deep/unrelated.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["*.side.js"] + } + `, + }, + dce: true, + run: { + stdout: "root kept\nnested kept\ndeeper kept\ndone", + }, + }); + itBundled("dce/PackageJsonSideEffectsBareNameMatchesNested", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/polyfill.js"; + import "demo-pkg/comp/polyfill.js"; + import "demo-pkg/comp/other.js"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/polyfill.js": `console.log("root poly kept")`, + "/Users/user/project/node_modules/demo-pkg/comp/polyfill.js": `console.log("nested poly kept")`, + "/Users/user/project/node_modules/demo-pkg/comp/other.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["polyfill.js"] + } + `, + }, + dce: true, + run: { + stdout: "root poly kept\nnested poly kept\ndone", + }, + }); + itBundled("dce/PackageJsonSideEffectsMixedNoSlashAndPath", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/lib/effect.side.js"; + import "demo-pkg/lib/specific.js"; + import "demo-pkg/comp/polyfill.js"; + import "demo-pkg/lib/drop.js"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/lib/effect.side.js": `console.log("glob kept")`, + "/Users/user/project/node_modules/demo-pkg/lib/specific.js": `console.log("exact kept")`, + "/Users/user/project/node_modules/demo-pkg/comp/polyfill.js": `console.log("bare kept")`, + "/Users/user/project/node_modules/demo-pkg/lib/drop.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["*.side.js", "./lib/specific.js", "polyfill.js"] + } + `, + }, + dce: true, + run: { + stdout: "glob kept\nexact kept\nbare kept\ndone", + }, + }); + itBundled("dce/PackageJsonSideEffectsDotSlashStaysRootOnly", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/polyfill.js"; + import "demo-pkg/comp/polyfill.js"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/polyfill.js": `console.log("root kept")`, + "/Users/user/project/node_modules/demo-pkg/comp/polyfill.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["./polyfill.js"] + } + `, + }, + dce: true, + run: { + stdout: "root kept\ndone", + }, + }); itBundled("dce/PackageJsonSideEffectsGlobInvalidPattern", { files: { "/Users/user/project/src/entry.js": /* js */ ` From 126025727ceb71723da44373710218549fd667c4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:56:31 +0000 Subject: [PATCH 02/10] Remove .css skip from sideEffects glob parser Now that slashless entries always route through the glob branch, a bare 'style.css' entry would be filtered out at parse time, producing an empty glob list. Bun keeps CSS imports regardless of the sideEffects field so this was not observable in the bundle output, but the parser should faithfully represent what package.json says rather than diverge between 'style.css' and './style.css'. Drop the skip from both glob branches. --- src/resolver/package_json.rs | 10 ---------- test/bundler/esbuild/dce.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index a50b9223da71..578b0c37a29c 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -732,11 +732,6 @@ impl PackageJSON { for item in items { if let Some(name) = item.as_str() { - // Skip CSS files as they're not relevant for tree-shaking - if bun_paths::extension(name) == b".css" { - continue; - } - let slashless = !strings::contains_char(name, b'/'); // Store the pattern relative to the package directory @@ -772,11 +767,6 @@ impl PackageJSON { glob_list.reserve(items.len()); for item in items { if let Some(name) = item.as_str() { - // Skip CSS files as they're not relevant for tree-shaking - if bun_paths::extension(name) == b".css" { - continue; - } - // Store the pattern relative to the package directory let joined: [&[u8]; 3] = [ json_source.path.name().dir_with_trailing_slash(), diff --git a/test/bundler/esbuild/dce.test.ts b/test/bundler/esbuild/dce.test.ts index a5ae6e0ec78e..85d17310edad 100644 --- a/test/bundler/esbuild/dce.test.ts +++ b/test/bundler/esbuild/dce.test.ts @@ -898,6 +898,33 @@ describe("bundler", () => { stdout: "root kept\ndone", }, }); + itBundled("dce/PackageJsonSideEffectsBareCSSAndJS", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/style.css"; + import "demo-pkg/setup.js"; + import "demo-pkg/lib/setup.js"; + import "demo-pkg/lib/other.js"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/style.css": `body { color: red; }`, + "/Users/user/project/node_modules/demo-pkg/setup.js": `console.log("root setup kept")`, + "/Users/user/project/node_modules/demo-pkg/lib/setup.js": `console.log("nested setup kept")`, + "/Users/user/project/node_modules/demo-pkg/lib/other.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["style.css", "setup.js"] + } + `, + }, + outdir: "/out", + run: { + stdout: "root setup kept\nnested setup kept\ndone", + }, + onAfterBundle(api) { + api.expectFile("/out/entry.css").toContain("color: red"); + }, + }); itBundled("dce/PackageJsonSideEffectsGlobInvalidPattern", { files: { "/Users/user/project/src/entry.js": /* js */ ` From cd0fabb7ee05fe94867abca9183c0f0c621985f7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:57:18 +0000 Subject: [PATCH 03/10] Shorten sideEffects classifier comment --- src/resolver/package_json.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index 578b0c37a29c..754e5d69bfc9 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -703,10 +703,8 @@ impl PackageJSON { let mut has_globs = false; let mut has_exact = false; - // First pass: check if we have glob patterns and exact patterns. - // Per webpack's documented semantics for this field, patterns - // without a '/' match against the basename at any depth (as if - // prefixed with "**/"), so they are always glob patterns. + // Webpack matches slashless patterns as "**/", so they + // are always globs. for item in items { if let Some(name) = item.as_str() { if !strings::contains_char(name, b'/') From 4359bad030638344e526b320aa883872a79845ed Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:58:21 +0000 Subject: [PATCH 04/10] Fit sideEffects classifier comment on one line --- src/resolver/package_json.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index 754e5d69bfc9..895d4904781d 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -703,8 +703,7 @@ impl PackageJSON { let mut has_globs = false; let mut has_exact = false; - // Webpack matches slashless patterns as "**/", so they - // are always globs. + // Webpack matches slashless patterns as "**/", so they are always globs. for item in items { if let Some(name) = item.as_str() { if !strings::contains_char(name, b'/') From 4990da18e4fcf96ae89682a7a07db9963daa3c82 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:11:36 +0000 Subject: [PATCH 05/10] Enable dce FAIL grep for the CSS+JS sideEffects test --- test/bundler/esbuild/dce.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/bundler/esbuild/dce.test.ts b/test/bundler/esbuild/dce.test.ts index 85d17310edad..688aecb8d5d0 100644 --- a/test/bundler/esbuild/dce.test.ts +++ b/test/bundler/esbuild/dce.test.ts @@ -918,6 +918,8 @@ describe("bundler", () => { `, }, outdir: "/out", + dce: true, + dceKeepMarkerCount: false, run: { stdout: "root setup kept\nnested setup kept\ndone", }, From 09482add8d5fab2f5c9858a40e965622761a1d0a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:38:25 +0000 Subject: [PATCH 06/10] resolver: match sideEffects entries without "/" in every directory An entry without "/" lists a file in every directory of the package, as in webpack and esbuild. Every entry was joined onto the package directory, so "*.side.js" and "polyfill.js" listed root files only and `bun build` left the nested files out of the bundle. A plain file name goes in a set keyed by file name, which is one hash lookup for each resolved file at any list size. A glob without "/" is stored as "**/", with no directory in the pattern. An entry with "/" or "\" is stored as before, and no list changes its variant. One function reads the array now. It replaces three copies of the join, two copies of the glob test and two copies of the ".css" skip. A list that has no pattern left to match no longer copies the path on each lookup. --- src/resolver/package_json.rs | 283 +++++++++++++++-------------- test/bundler/esbuild/dce.test.ts | 278 ++++++++++++++++++++++++++-- test/bundler/esbuild/extra.test.ts | 2 - test/bundler/expectBundled.ts | 5 - 4 files changed, 413 insertions(+), 155 deletions(-) diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index dd4af26953fd..bcb2fd349853 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -228,7 +228,7 @@ pub enum SideEffects { /// "sideEffects": false False, /// "sideEffects": ["file.js", "other.js"] - Map(SideEffectsMap), + Map(ExactEntries), /// "sideEffects": ["side_effects/*.js"] Glob(GlobList), /// "sideEffects": ["file.js", "side_effects/*.js"] - mixed patterns @@ -239,51 +239,163 @@ type SideEffectsMap = bun_collections::HashMap; type GlobList = Vec>>; +/// The entries that have no glob syntax. +#[derive(Default)] +pub struct ExactEntries { + /// Entries with a path separator, joined onto the package directory. + paths: SideEffectsMap, + /// Entries without one. In a box, so that a `PackageJSON` stays in its allocator size class. + names: Option>, +} + +/// Entries that are a file name. Each lists that file in every directory of the package. +struct FileNames { + /// The package directory with its trailing separator, as the keys of `ExactEntries::paths` spell it. + directory: Box<[u8]>, + names: SideEffectsMap, +} + pub struct MixedPatterns { - pub(crate) exact: SideEffectsMap, + pub(crate) exact: ExactEntries, pub(crate) globs: GlobList, } +impl ExactEntries { + fn contains(&self, path: &[u8]) -> bool { + if !self.paths.is_empty() + && self + .paths + .contains_key(&StringHashMapUnownedKey::init(path)) + { + return true; + } + self.names + .as_deref() + .is_some_and(|names| names.contains(path)) + } +} + +impl FileNames { + fn contains(&self, path: &[u8]) -> bool { + path.strip_prefix(&*self.directory).is_some_and(|below| { + let name = bun_paths::basename(below); + self.names + .contains_key(&StringHashMapUnownedKey::init(name)) + }) + } +} + +/// Returns true if an entry of "sideEffects" has glob syntax. +fn is_glob(entry: &[u8]) -> bool { + strings::contains_any(entry, b"*?[{") || glob::has_extglob_prefix(entry) +} + +fn any_glob_matches(globs: &GlobList, path: &[u8]) -> bool { + if globs.is_empty() { + return false; + } + // Normalize path for cross-platform glob matching + let Ok(normalized_path) = PackageJSON::normalize_path_for_glob(path) else { + return true; + }; + globs + .iter() + .any(|pattern| pattern.r#match(&normalized_path).matches()) +} + impl SideEffects { pub(crate) fn has_side_effects(&self, path: &[u8]) -> bool { match self { SideEffects::Unspecified => true, SideEffects::False => false, - SideEffects::Map(map) => map.contains_key(&StringHashMapUnownedKey::init(path)), - SideEffects::Glob(glob_list) => { - // Normalize path for cross-platform glob matching - let Ok(normalized_path) = PackageJSON::normalize_path_for_glob(path) else { - return true; - }; + SideEffects::Map(exact) => exact.contains(path), + SideEffects::Glob(globs) => any_glob_matches(globs, path), + SideEffects::Mixed(mixed) => { + mixed.exact.contains(path) || any_glob_matches(&mixed.globs, path) + } + } + } - for pattern in glob_list.iter() { - if pattern.r#match(&normalized_path).matches() { - return true; - } - } - false + /// Reads the array that "sideEffects" holds in the package.json in `package_dir`. + /// + /// An entry without a path separator lists a file in every directory of the package, as in + /// webpack and esbuild. Any other entry is a path from `package_dir`. + fn from_array( + items: &[js_ast::E::JsonValue], + package_dir: &[u8], + r_fs: &fs::FileSystem, + ) -> SideEffects { + let entries = || items.iter().filter_map(|item| item.as_str()); + let glob_count = entries().filter(|entry| is_glob(entry)).count(); + let exact_count = entries().count() - glob_count; + + let mut exact = ExactEntries::default(); + let mut globs = GlobList::default(); + let mut spill: Vec = Vec::new(); + + for entry in entries() { + // A list with a glob has never stored its `.css` entries. The JS that imports the CSS depends on it. + if glob_count > 0 && bun_paths::extension(entry) == b".css" { + continue; } - SideEffects::Mixed(mixed) => { - // First check exact matches - if mixed - .exact - .contains_key(&StringHashMapUnownedKey::init(path)) - { - return true; - } - // Then check glob patterns with normalized path - let Ok(normalized_path) = PackageJSON::normalize_path_for_glob(path) else { - return true; + let is_file_name = !strings::contains_any(entry, b"/\\"); + + if is_glob(entry) { + let pattern = if is_file_name { + // No directory in the pattern: its name is not glob syntax. + [&b"**/"[..], entry].concat() + } else { + let mut joined = r_fs.join(&mut spill, &[package_dir, entry]).to_vec(); + // Normalize pattern to use forward slashes for cross-platform compatibility + bun_paths::slashes_to_posix_in_place(&mut joined[..]); + joined }; + if globs.is_empty() { + globs.reserve(glob_count); + } + globs.push(glob::Prepared::new(pattern.into_boxed_slice())); + continue; + } - for pattern in mixed.globs.iter() { - if pattern.r#match(&normalized_path).matches() { - return true; + let joined = r_fs.join(&mut spill, &[package_dir, entry]); + // The join resolves `.` and `..`, so those are paths. + let directory = joined.strip_suffix(entry).filter(|directory| { + is_file_name + && directory + .last() + .is_some_and(|&last| bun_paths::is_sep_native(last)) + }); + match directory { + Some(directory) => { + let file_names = exact.names.get_or_insert_with(|| { + Box::new(FileNames { + directory: directory.into(), + names: SideEffectsMap::with_capacity(exact_count), + }) + }); + debug_assert!(*file_names.directory == *directory); + let _ = file_names + .names + .insert(StringHashMapUnownedKey::init(entry), ()); + } + None => { + if exact.paths.is_empty() { + exact.paths.reserve(exact_count); } + let _ = exact + .paths + .insert(StringHashMapUnownedKey::init(joined), ()); } - false } } + + match (glob_count > 0, exact_count > 0) { + // If the array is empty, treat it as false (no side effects) + (false, false) => SideEffects::False, + (false, true) => SideEffects::Map(exact), + (true, false) => SideEffects::Glob(globs), + (true, true) => SideEffects::Mixed(MixedPatterns { exact, globs }), + } } } @@ -704,114 +816,11 @@ impl PackageJSON { package_json.side_effects = SideEffects::False; } } else if let js_ast::ExprData::EArrayJSON(e_array) = &side_effects_field.data { - // Handle arrays, including empty arrays - let items = e_array.get().items(); - let mut map = SideEffectsMap::default(); - let mut glob_list = GlobList::default(); - let mut has_globs = false; - let mut has_exact = false; - - // Webpack matches slashless patterns as "**/", so they are always globs. - for item in items { - if let Some(name) = item.as_str() { - if !strings::contains_char(name, b'/') - || strings::contains_char(name, b'*') - || strings::contains_char(name, b'?') - || strings::contains_char(name, b'[') - || strings::contains_char(name, b'{') - || glob::has_extglob_prefix(name) - { - has_globs = true; - } else { - has_exact = true; - } - } - } - - let mut spill: Vec = Vec::new(); - - // If the array is empty, treat it as false (no side effects) - if !has_globs && !has_exact { - package_json.side_effects = SideEffects::False; - } else if has_globs && has_exact { - // Mixed patterns - use both exact and glob matching - map.reserve(items.len()); - glob_list.reserve(items.len()); - - for item in items { - if let Some(name) = item.as_str() { - let slashless = !strings::contains_char(name, b'/'); - - // Store the pattern relative to the package directory - let joined: [&[u8]; 3] = [ - json_source.path.name().dir_with_trailing_slash(), - if slashless { b"**/" } else { b"" }, - name, - ]; - - let pattern = r_fs.join(&mut spill, &joined); - - if slashless - || strings::contains_char(name, b'*') - || strings::contains_char(name, b'?') - || strings::contains_char(name, b'[') - || strings::contains_char(name, b'{') - || glob::has_extglob_prefix(name) - { - // Normalize pattern to use forward slashes for cross-platform compatibility - let normalized_pattern = Self::normalize_path_for_glob(pattern) - .unwrap_or_else(|_| pattern.to_vec()); - glob_list.push(glob::Prepared::new( - normalized_pattern.into_boxed_slice(), - )); - } else { - let _ = map.insert(StringHashMapUnownedKey::init(pattern), ()); - } - } - } - package_json.side_effects = SideEffects::Mixed(MixedPatterns { - exact: map, - globs: glob_list, - }); - } else if has_globs { - // Only glob patterns - glob_list.reserve(items.len()); - for item in items { - if let Some(name) = item.as_str() { - // Store the pattern relative to the package directory - let joined: [&[u8]; 3] = [ - json_source.path.name().dir_with_trailing_slash(), - if strings::contains_char(name, b'/') { - b"" - } else { - b"**/" - }, - name, - ]; - - let pattern = r_fs.join(&mut spill, &joined); - // Normalize pattern to use forward slashes for cross-platform compatibility - let normalized_pattern = Self::normalize_path_for_glob(pattern) - .unwrap_or_else(|_| pattern.to_vec()); - glob_list - .push(glob::Prepared::new(normalized_pattern.into_boxed_slice())); - } - } - package_json.side_effects = SideEffects::Glob(glob_list); - } else { - // Only exact matches - map.reserve(items.len()); - for item in items { - if let Some(name) = item.as_str() { - let joined: [&[u8]; 2] = - [json_source.path.name().dir_with_trailing_slash(), name]; - - let pattern = r_fs.join(&mut spill, &joined); - let _ = map.insert(StringHashMapUnownedKey::init(pattern), ()); - } - } - package_json.side_effects = SideEffects::Map(map); - } + package_json.side_effects = SideEffects::from_array( + e_array.get().items(), + json_source.path.name().dir_with_trailing_slash(), + r_fs, + ); } } diff --git a/test/bundler/esbuild/dce.test.ts b/test/bundler/esbuild/dce.test.ts index 9220b21de2ba..1e103a9a0b31 100644 --- a/test/bundler/esbuild/dce.test.ts +++ b/test/bundler/esbuild/dce.test.ts @@ -1,4 +1,5 @@ -import { describe, expect } from "bun:test"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isASAN, isDebug, isWindows, tempDir } from "harness"; import { dedent, itBundled } from "../expectBundled"; // Tests ported from: @@ -801,8 +802,40 @@ describe("bundler", () => { stdout: "main effect - should be preserved\ndone", }, }); - // Webpack (which defines this field) documents that patterns without a '/' - // match at any depth, like "**/pattern". esbuild follows the same rule. + // An entry without "/" lists a file in every directory of the package, as in webpack and esbuild. + for (const backend of ["cli", "api"] as const) { + itBundled(`dce/PackageJsonSideEffectsNoSlashAnyDirectory-${backend}`, { + backend, + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/root.side.js"; + import "demo-pkg/deep/nested.side.js"; + import "demo-pkg/.hidden/x.side.js"; + import "demo-pkg/deep/.dot.side.js"; + import "demo-pkg/polyfill.js"; + import "demo-pkg/comp/polyfill.js"; + import "demo-pkg/deep/unlisted.js"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/root.side.js": `console.log("root glob")`, + "/Users/user/project/node_modules/demo-pkg/deep/nested.side.js": `console.log("nested glob")`, + "/Users/user/project/node_modules/demo-pkg/.hidden/x.side.js": `console.log("dot directory")`, + "/Users/user/project/node_modules/demo-pkg/deep/.dot.side.js": `console.log("dotfile")`, + "/Users/user/project/node_modules/demo-pkg/polyfill.js": `console.log("root name")`, + "/Users/user/project/node_modules/demo-pkg/comp/polyfill.js": `console.log("nested name")`, + "/Users/user/project/node_modules/demo-pkg/deep/unlisted.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["*.side.js", "polyfill.js"] + } + `, + }, + dce: true, + run: { + stdout: "root glob\nnested glob\ndot directory\ndotfile\nroot name\nnested name\ndone", + }, + }); + } itBundled("dce/PackageJsonSideEffectsGlobNoSlashMatchesNested", { files: { "/Users/user/project/src/entry.js": /* js */ ` @@ -873,24 +906,25 @@ describe("bundler", () => { stdout: "glob kept\nexact kept\nbare kept\ndone", }, }); - itBundled("dce/PackageJsonSideEffectsDotSlashStaysRootOnly", { + itBundled("dce/PackageJsonSideEffectsStarListsEveryFile", { files: { "/Users/user/project/src/entry.js": /* js */ ` - import "demo-pkg/polyfill.js"; - import "demo-pkg/comp/polyfill.js"; + import "demo-pkg/a.js"; + import "demo-pkg/deep/b.js"; + import "demo-pkg/deep/.hidden/c.js"; console.log("done"); `, - "/Users/user/project/node_modules/demo-pkg/polyfill.js": `console.log("root kept")`, - "/Users/user/project/node_modules/demo-pkg/comp/polyfill.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/a.js": `console.log("a")`, + "/Users/user/project/node_modules/demo-pkg/deep/b.js": `console.log("b")`, + "/Users/user/project/node_modules/demo-pkg/deep/.hidden/c.js": `console.log("c")`, "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` { - "sideEffects": ["./polyfill.js"] + "sideEffects": ["*"] } `, }, - dce: true, run: { - stdout: "root kept\ndone", + stdout: "a\nb\nc\ndone", }, }); itBundled("dce/PackageJsonSideEffectsBareCSSAndJS", { @@ -922,6 +956,228 @@ describe("bundler", () => { api.expectFile("/out/entry.css").toContain("color: red"); }, }); + // A package.json without a name gives the last answer only when its list has no glob. With a glob, the nearest + // package.json that has a name answers again. So this keeps the nested file only if a file name is not a glob. + itBundled("dce/PackageJsonSideEffectsFileNameInPackageWithoutName", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "no-name/polyfill.js"; + import "no-name/sub/polyfill.js"; + import "no-name/sub/unlisted.js"; + console.log("done"); + `, + "/Users/user/project/package.json": /* json */ ` + { + "name": "app", + "sideEffects": false + } + `, + "/Users/user/project/node_modules/no-name/polyfill.js": `console.log("root name")`, + "/Users/user/project/node_modules/no-name/sub/polyfill.js": `console.log("nested name")`, + "/Users/user/project/node_modules/no-name/sub/unlisted.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/no-name/package.json": /* json */ ` + { + "sideEffects": ["polyfill.js"] + } + `, + }, + dce: true, + run: { + stdout: "root name\nnested name\ndone", + }, + }); + // "[slug]" in the path of the package is not a character class for an entry without "/". + itBundled("dce/PackageJsonSideEffectsNoSlashUnderBracketDirectory", { + files: { + "/Users/user/project/[slug]/src/entry.js": /* js */ ` + import "demo-pkg/root.side.js"; + import "demo-pkg/deep/nested.side.js"; + import "demo-pkg/polyfill.js"; + import "demo-pkg/deep/polyfill.js"; + import "demo-pkg/deep/unlisted.js"; + console.log("done"); + `, + "/Users/user/project/[slug]/node_modules/demo-pkg/root.side.js": `console.log("root glob")`, + "/Users/user/project/[slug]/node_modules/demo-pkg/deep/nested.side.js": `console.log("nested glob")`, + "/Users/user/project/[slug]/node_modules/demo-pkg/polyfill.js": `console.log("root name")`, + "/Users/user/project/[slug]/node_modules/demo-pkg/deep/polyfill.js": `console.log("nested name")`, + "/Users/user/project/[slug]/node_modules/demo-pkg/deep/unlisted.js": `console.log("TEST FAILED")`, + "/Users/user/project/[slug]/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["*.side.js", "polyfill.js"] + } + `, + }, + dce: true, + run: { + stdout: "root glob\nnested glob\nroot name\nnested name\ndone", + }, + }); + // An entry with "/" is a path from the directory of the package.json. + itBundled("dce/PackageJsonSideEffectsEntryWithSlashIsAPath", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/polyfill.js"; + import "demo-pkg/lib/polyfill.js"; + import "demo-pkg/root.side.js"; + import "demo-pkg/lib/nested.side.js"; + import "demo-pkg/comp/setup.js"; + import "demo-pkg/lib/comp/setup.js"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/polyfill.js": `console.log("root name")`, + "/Users/user/project/node_modules/demo-pkg/lib/polyfill.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/root.side.js": `console.log("root glob")`, + "/Users/user/project/node_modules/demo-pkg/lib/nested.side.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/comp/setup.js": `console.log("comp path")`, + "/Users/user/project/node_modules/demo-pkg/lib/comp/setup.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["./polyfill.js", "./*.side.js", "comp/setup.js"] + } + `, + }, + dce: true, + run: { + stdout: "root name\nroot glob\ncomp path\ndone", + }, + }); + // "\\" separates directories in an entry on Windows only. It is not part of a file name on any platform. + itBundled("dce/PackageJsonSideEffectsEntryWithBackslashIsAPath", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/sub/effect.js"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/sub/effect.js": `console.log("sub effect")`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["sub\\\\effect.js"] + } + `, + }, + run: { + stdout: isWindows ? "sub effect\ndone" : "done", + }, + }); + itBundled("dce/PackageJsonSideEffectsExactListInPackageWithoutName", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "no-name/listed.js"; + import "no-name/unlisted.js"; + console.log("done"); + `, + "/Users/user/project/package.json": /* json */ ` + { + "name": "app", + "sideEffects": false + } + `, + "/Users/user/project/node_modules/no-name/listed.js": `console.log("listed")`, + "/Users/user/project/node_modules/no-name/unlisted.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/no-name/package.json": /* json */ ` + { + "sideEffects": ["./listed.js"] + } + `, + }, + dce: true, + run: { + stdout: "listed\ndone", + }, + }); + // A list that has a glob does not store its ".css" entries. A list without a glob stores them. The CSS is in + // the bundle either way. A lazily loaded importer keeps an empty initializer for a CSS file with side effects. + itBundled("dce/PackageJsonSideEffectsCSSEntries", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + console.log(require("with-glob").name, require("with-both").name, require("with-path").name); + `, + "/Users/user/project/node_modules/with-glob/index.js": /* js */ ` + import "./sub/globbed.css"; + export const name = "glob"; + `, + "/Users/user/project/node_modules/with-glob/sub/globbed.css": `.globbed { color: red; }`, + "/Users/user/project/node_modules/with-glob/package.json": /* json */ ` + { + "sideEffects": ["*.css"] + } + `, + "/Users/user/project/node_modules/with-both/index.js": /* js */ ` + import "./sub/mixed.css"; + export const name = "both"; + `, + "/Users/user/project/node_modules/with-both/sub/mixed.css": `.mixed { color: green; }`, + "/Users/user/project/node_modules/with-both/package.json": /* json */ ` + { + "sideEffects": ["./sub/mixed.css", "./*.side.js"] + } + `, + "/Users/user/project/node_modules/with-path/index.js": /* js */ ` + import "./sub/exact.css"; + export const name = "path"; + `, + "/Users/user/project/node_modules/with-path/sub/exact.css": `.exact { color: blue; }`, + "/Users/user/project/node_modules/with-path/package.json": /* json */ ` + { + "sideEffects": ["./sub/exact.css"] + } + `, + }, + outdir: "/out", + run: { + stdout: "glob both path", + }, + onAfterBundle(api) { + const js = api.readFile("/out/entry.js"); + expect(js).not.toContain("init_globbed"); + expect(js).not.toContain("init_mixed"); + expect(js).toContain("init_exact"); + const css = api.readFile("/out/entry.css"); + expect(css).toContain(".globbed"); + expect(css).toContain(".mixed"); + expect(css).toContain(".exact"); + }, + }); + // The resolver asks the "sideEffects" list about every file that it resolves in the package. A file name in the + // list costs that ask one hash lookup, as a path does. A glob costs it one match: spelled "**/m0.js", these + // 20,000 names take 10.8 s of CPU on a debug build, against 0.2 s. + test("dce/PackageJsonSideEffectsFileNamesCostOneLookup", async () => { + const files = isDebug || isASAN ? 20 : 400; + const pkg = (name: string, spell: (i: number) => string) => ({ + "package.json": JSON.stringify({ + name, + version: "1.0.0", + main: "index.js", + sideEffects: Array.from({ length: 20_000 }, (_, i) => spell(i)), + }), + "index.js": + Array.from({ length: files }, (_, i) => `import "./unlisted${i}.js";\n`).join("") + "export const x = 1;\n", + ...Object.fromEntries(Array.from({ length: files }, (_, i) => [`unlisted${i}.js`, `globalThis.x${i} = 1;\n`])), + }); + using dir = tempDir("side-effects-file-names", { + "names.js": `import { x } from "names"; console.log(x);`, + "paths.js": `import { x } from "paths"; console.log(x);`, + node_modules: { + names: pkg("names", i => `m${i}.js`), + paths: pkg("paths", i => `./m${i}.js`), + }, + }); + const userCpu = async (entry: string) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "build", entry, "--outdir=out"], + cwd: String(dir), + env: bunEnv, + stdout: "ignore", + stderr: "inherit", + }); + expect(await proc.exited).toBe(0); + return Number(proc.resourceUsage()!.cpuTime.user) / 1000; + }; + const paths = await userCpu("paths.js"); + const names = await userCpu("names.js"); + expect(names).toBeLessThan(paths * 4 + 100); + }); itBundled("dce/PackageJsonSideEffectsGlobInvalidPattern", { files: { "/Users/user/project/src/entry.js": /* js */ ` diff --git a/test/bundler/esbuild/extra.test.ts b/test/bundler/esbuild/extra.test.ts index 7cd5621a0b63..6db05156de97 100644 --- a/test/bundler/esbuild/extra.test.ts +++ b/test/bundler/esbuild/extra.test.ts @@ -1314,7 +1314,6 @@ describe("bundler", () => { "foo/dir/x.js": `global.dce6 = 123`, "foo/package.json": `{ "main": "dir/x", "sideEffects": ["x.*"] }`, }, - skipIfWeDidNotImplementWildcardSideEffects: true, run: true, }); itBundled(`extra/TreeShaking9`, { @@ -1323,7 +1322,6 @@ describe("bundler", () => { "foo/dir/x.js": `global.dce6 = 123`, "foo/package.json": `{ "main": "dir/x", "sideEffects": ["**/x.*"] }`, }, - skipIfWeDidNotImplementWildcardSideEffects: true, run: true, }); itBundled(`extra/TreeShaking10`, { diff --git a/test/bundler/expectBundled.ts b/test/bundler/expectBundled.ts index ad5ccafccb1e..0b9d1e4cd863 100644 --- a/test/bundler/expectBundled.ts +++ b/test/bundler/expectBundled.ts @@ -344,9 +344,6 @@ export interface BundlerTestInput { /** Run after bundle happens but before runtime. */ onAfterBundle?(api: BundlerTestBundleAPI): Promise | void; - /* TODO: remove this from the tests after this is implemented */ - skipIfWeDidNotImplementWildcardSideEffects?: boolean; - snapshotSourceMap?: Record; expectExactFilesize?: Record; @@ -575,7 +572,6 @@ function expectBundled( mangleProps, mangleQuoted, nodePaths, - skipIfWeDidNotImplementWildcardSideEffects, stdin, targetFromAPI, mode, @@ -634,7 +630,6 @@ function expectBundled( mangleProps, mangleQuoted, nodePaths, - skipIfWeDidNotImplementWildcardSideEffects, stdin, targetFromAPI, })) { From 8b827fb89ba0f307c3994f9f468ec3bfb6240aba Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 23 Oct 2026 20:38:28 +0000 Subject: [PATCH 07/10] resolver: give each half of the sideEffects lookup one call site The exact half and the glob half of `has_side_effects` each have one call site now, so both are inlined into the lookup as they were before this branch. The file-name probe stays out of line. A lookup in a list of paths takes 155 instructions in a release build, down from 171 with the two call sites. It takes 140 on main. --- src/resolver/package_json.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index bcb2fd349853..55dc423143ad 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -276,6 +276,8 @@ impl ExactEntries { } impl FileNames { + // Out of line, so that a list without file names keeps the short lookup it had. + #[inline(never)] fn contains(&self, path: &[u8]) -> bool { path.strip_prefix(&*self.directory).is_some_and(|below| { let name = bun_paths::basename(below); @@ -290,7 +292,7 @@ fn is_glob(entry: &[u8]) -> bool { strings::contains_any(entry, b"*?[{") || glob::has_extglob_prefix(entry) } -fn any_glob_matches(globs: &GlobList, path: &[u8]) -> bool { +fn any_glob_matches(globs: &[glob::Prepared>], path: &[u8]) -> bool { if globs.is_empty() { return false; } @@ -305,15 +307,14 @@ fn any_glob_matches(globs: &GlobList, path: &[u8]) -> bool { impl SideEffects { pub(crate) fn has_side_effects(&self, path: &[u8]) -> bool { - match self { - SideEffects::Unspecified => true, - SideEffects::False => false, - SideEffects::Map(exact) => exact.contains(path), - SideEffects::Glob(globs) => any_glob_matches(globs, path), - SideEffects::Mixed(mixed) => { - mixed.exact.contains(path) || any_glob_matches(&mixed.globs, path) - } - } + let (exact, globs): (Option<&ExactEntries>, &[glob::Prepared>]) = match self { + SideEffects::Unspecified => return true, + SideEffects::False => return false, + SideEffects::Map(exact) => (Some(exact), &[]), + SideEffects::Glob(globs) => (None, globs), + SideEffects::Mixed(mixed) => (Some(&mixed.exact), &mixed.globs), + }; + exact.is_some_and(|exact| exact.contains(path)) || any_glob_matches(globs, path) } /// Reads the array that "sideEffects" holds in the package.json in `package_dir`. From 39ee8b2391ace60a7b6706b5ebac842da5066334 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 24 Oct 2026 05:45:11 +0000 Subject: [PATCH 08/10] resolver: key sideEffects file names in the one exact set A file name is now one more key of the exact set, with two flags that say which kinds of key the set holds. The separate boxed set and its copy of the package directory are gone, and so is the check that a named file is inside the package directory. An entry without "/" has no directory: a name and a glob now give the same answer for a file that "main" reaches outside the package, and both keep it. `SideEffects` is 80 bytes and `PackageJSON` is 632 bytes again, as on main, and a list with a file name allocates what it did on main. Tests: the CPU-time case is removed. It ran close to the default timeout on a debug build. New cases reach a listed file through a package that has a name, through the project's own package.json, through a barrel re-export, through "main" outside the package and through an onLoad plugin. --- src/resolver/package_json.rs | 165 +++++++++----------------- test/bundler/bundler_barrel.test.ts | 35 ++++++ test/bundler/esbuild/dce.test.ts | 177 ++++++++++++++++++++-------- 3 files changed, 222 insertions(+), 155 deletions(-) diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index 55dc423143ad..9b412c306d9d 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -227,7 +227,7 @@ pub enum SideEffects { Unspecified, /// "sideEffects": false False, - /// "sideEffects": ["file.js", "other.js"] + /// "sideEffects": ["file.js", "./other.js"] Map(ExactEntries), /// "sideEffects": ["side_effects/*.js"] Glob(GlobList), @@ -237,22 +237,16 @@ pub enum SideEffects { type SideEffectsMap = bun_collections::HashMap; -type GlobList = Vec>>; +type GlobList = Box<[glob::Prepared>]>; -/// The entries that have no glob syntax. +/// The entries that have no glob syntax, as one set of keys: a resolved file costs one lookup, however long the list is. #[derive(Default)] pub struct ExactEntries { - /// Entries with a path separator, joined onto the package directory. - paths: SideEffectsMap, - /// Entries without one. In a box, so that a `PackageJSON` stays in its allocator size class. - names: Option>, -} - -/// Entries that are a file name. Each lists that file in every directory of the package. -struct FileNames { - /// The package directory with its trailing separator, as the keys of `ExactEntries::paths` spell it. - directory: Box<[u8]>, - names: SideEffectsMap, + keys: SideEffectsMap, + /// An entry with a path separator is keyed by its path, joined onto the package directory. + has_paths: bool, + /// An entry without one is keyed as written. It is a file name, and lists that file in every directory. + has_names: bool, } pub struct MixedPatterns { @@ -261,33 +255,13 @@ pub struct MixedPatterns { } impl ExactEntries { + #[inline] fn contains(&self, path: &[u8]) -> bool { - if !self.paths.is_empty() - && self - .paths - .contains_key(&StringHashMapUnownedKey::init(path)) - { - return true; - } - self.names - .as_deref() - .is_some_and(|names| names.contains(path)) - } -} - -impl FileNames { - // Out of line, so that a list without file names keeps the short lookup it had. - #[inline(never)] - fn contains(&self, path: &[u8]) -> bool { - path.strip_prefix(&*self.directory).is_some_and(|below| { - let name = bun_paths::basename(below); - self.names - .contains_key(&StringHashMapUnownedKey::init(name)) - }) + let has = |key: &[u8]| self.keys.contains_key(&StringHashMapUnownedKey::init(key)); + (self.has_paths && has(path)) || (self.has_names && has(bun_paths::basename(path))) } } -/// Returns true if an entry of "sideEffects" has glob syntax. fn is_glob(entry: &[u8]) -> bool { strings::contains_any(entry, b"*?[{") || glob::has_extglob_prefix(entry) } @@ -307,95 +281,72 @@ fn any_glob_matches(globs: &[glob::Prepared>], path: &[u8]) -> bool { impl SideEffects { pub(crate) fn has_side_effects(&self, path: &[u8]) -> bool { - let (exact, globs): (Option<&ExactEntries>, &[glob::Prepared>]) = match self { - SideEffects::Unspecified => return true, - SideEffects::False => return false, - SideEffects::Map(exact) => (Some(exact), &[]), - SideEffects::Glob(globs) => (None, globs), - SideEffects::Mixed(mixed) => (Some(&mixed.exact), &mixed.globs), - }; - exact.is_some_and(|exact| exact.contains(path)) || any_glob_matches(globs, path) + match self { + SideEffects::Unspecified => true, + SideEffects::False => false, + SideEffects::Map(exact) => exact.contains(path), + SideEffects::Glob(globs) => any_glob_matches(globs, path), + SideEffects::Mixed(mixed) => { + mixed.exact.contains(path) || any_glob_matches(&mixed.globs, path) + } + } } - /// Reads the array that "sideEffects" holds in the package.json in `package_dir`. - /// - /// An entry without a path separator lists a file in every directory of the package, as in - /// webpack and esbuild. Any other entry is a path from `package_dir`. + /// An entry without a path separator lists a file in every directory, as in webpack and esbuild. Any other is a path from `package_dir`. fn from_array( items: &[js_ast::E::JsonValue], package_dir: &[u8], r_fs: &fs::FileSystem, ) -> SideEffects { let entries = || items.iter().filter_map(|item| item.as_str()); - let glob_count = entries().filter(|entry| is_glob(entry)).count(); - let exact_count = entries().count() - glob_count; - - let mut exact = ExactEntries::default(); - let mut globs = GlobList::default(); + let has_globs = entries().any(is_glob); + let has_exact = entries().any(|entry| !is_glob(entry)); + // Only a list without a glob stores its `.css` entries. dce/PackageJsonSideEffectsCSSEntries pins what that changes. + let stored = + || entries().filter(|entry| !(has_globs && bun_paths::extension(entry) == b".css")); + let glob_count = stored().filter(|entry| is_glob(entry)).count(); + + let mut exact = ExactEntries { + keys: SideEffectsMap::with_capacity(stored().count() - glob_count), + ..Default::default() + }; + let mut globs = Vec::with_capacity(glob_count); let mut spill: Vec = Vec::new(); - for entry in entries() { - // A list with a glob has never stored its `.css` entries. The JS that imports the CSS depends on it. - if glob_count > 0 && bun_paths::extension(entry) == b".css" { - continue; - } - let is_file_name = !strings::contains_any(entry, b"/\\"); - - if is_glob(entry) { - let pattern = if is_file_name { - // No directory in the pattern: its name is not glob syntax. - [&b"**/"[..], entry].concat() + for entry in stored() { + let has_separator = strings::contains_any(entry, b"/\\"); + if !is_glob(entry) { + let key = if has_separator { + r_fs.join(&mut spill, &[package_dir, entry]) } else { - let mut joined = r_fs.join(&mut spill, &[package_dir, entry]).to_vec(); - // Normalize pattern to use forward slashes for cross-platform compatibility - bun_paths::slashes_to_posix_in_place(&mut joined[..]); - joined + entry }; - if globs.is_empty() { - globs.reserve(glob_count); - } - globs.push(glob::Prepared::new(pattern.into_boxed_slice())); + exact.has_paths |= has_separator; + exact.has_names |= !has_separator; + let _ = exact.keys.insert(StringHashMapUnownedKey::init(key), ()); continue; } - - let joined = r_fs.join(&mut spill, &[package_dir, entry]); - // The join resolves `.` and `..`, so those are paths. - let directory = joined.strip_suffix(entry).filter(|directory| { - is_file_name - && directory - .last() - .is_some_and(|&last| bun_paths::is_sep_native(last)) - }); - match directory { - Some(directory) => { - let file_names = exact.names.get_or_insert_with(|| { - Box::new(FileNames { - directory: directory.into(), - names: SideEffectsMap::with_capacity(exact_count), - }) - }); - debug_assert!(*file_names.directory == *directory); - let _ = file_names - .names - .insert(StringHashMapUnownedKey::init(entry), ()); - } - None => { - if exact.paths.is_empty() { - exact.paths.reserve(exact_count); - } - let _ = exact - .paths - .insert(StringHashMapUnownedKey::init(joined), ()); - } - } + let pattern = if has_separator { + let mut joined = r_fs.join(&mut spill, &[package_dir, entry]).to_vec(); + // Normalize pattern to use forward slashes for cross-platform compatibility + bun_paths::slashes_to_posix_in_place(&mut joined[..]); + joined + } else { + // No directory in the pattern, so the name of a directory is never glob syntax. + [&b"**/"[..], entry].concat() + }; + globs.push(glob::Prepared::new(pattern.into_boxed_slice())); } - match (glob_count > 0, exact_count > 0) { + match (has_globs, has_exact) { // If the array is empty, treat it as false (no side effects) (false, false) => SideEffects::False, (false, true) => SideEffects::Map(exact), - (true, false) => SideEffects::Glob(globs), - (true, true) => SideEffects::Mixed(MixedPatterns { exact, globs }), + (true, false) => SideEffects::Glob(globs.into_boxed_slice()), + (true, true) => SideEffects::Mixed(MixedPatterns { + exact, + globs: globs.into_boxed_slice(), + }), } } } diff --git a/test/bundler/bundler_barrel.test.ts b/test/bundler/bundler_barrel.test.ts index 5cb287c7b1b2..08dba30c2888 100644 --- a/test/bundler/bundler_barrel.test.ts +++ b/test/bundler/bundler_barrel.test.ts @@ -150,6 +150,41 @@ describe("bundler", () => { }, }); + // An entry without "/" lists the file in every directory of the package. + for (const [id, entry] of [ + ["FileName", "effect.js"], + ["GlobWithoutSlash", "effect.*"], + ]) { + itBundled(`barrel/SideEffectsArray${id}KeepsListedReExport`, { + files: { + "/entry.js": /* js */ ` + import { plain } from 'effectful'; + console.log(plain('x')); + `, + "/node_modules/effectful/package.json": JSON.stringify({ + name: "effectful", + main: "./src/index.js", + sideEffects: [entry], + }), + "/node_modules/effectful/src/index.js": /* js */ ` + export { plain } from './plain.js'; + export { TABLE } from './effect.js'; + `, + "/node_modules/effectful/src/plain.js": /* js */ ` + export const plain = s => s + "!"; + `, + "/node_modules/effectful/src/effect.js": /* js */ ` + export const TABLE = {}; + TABLE.marker = "THE_SIDE_EFFECT_RAN"; + `, + }, + outdir: "/out", + onAfterBundle(api) { + api.expectFile("/out/entry.js").toContain("THE_SIDE_EFFECT_RAN"); + }, + }); + } + // A module the array does not list is still tree-shaken away. itBundled("barrel/SideEffectsArrayDropsUnlistedReExport", { files: { diff --git a/test/bundler/esbuild/dce.test.ts b/test/bundler/esbuild/dce.test.ts index 1e103a9a0b31..37ecbb603910 100644 --- a/test/bundler/esbuild/dce.test.ts +++ b/test/bundler/esbuild/dce.test.ts @@ -1,5 +1,5 @@ -import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isDebug, isWindows, tempDir } from "harness"; +import { describe, expect } from "bun:test"; +import { isWindows } from "harness"; import { dedent, itBundled } from "../expectBundled"; // Tests ported from: @@ -956,8 +956,121 @@ describe("bundler", () => { api.expectFile("/out/entry.css").toContain("color: red"); }, }); - // A package.json without a name gives the last answer only when its list has no glob. With a glob, the nearest - // package.json that has a name answers again. So this keeps the nested file only if a file name is not a glob. + // The list of a package that has a name, reached through the index of the package. + itBundled("dce/PackageJsonSideEffectsNoSlashInNamedPackage", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import { v } from "@scope/ui"; + console.log(v); + `, + "/Users/user/project/node_modules/@scope/ui/index.js": /* js */ ` + import "./root.side.js"; + import "./sub/deep.side.js"; + import "./comp/polyfill.js"; + import "./sub/unlisted.js"; + export const v = "done"; + `, + "/Users/user/project/node_modules/@scope/ui/root.side.js": `console.log("root glob")`, + "/Users/user/project/node_modules/@scope/ui/sub/deep.side.js": `console.log("nested glob")`, + "/Users/user/project/node_modules/@scope/ui/comp/polyfill.js": `console.log("nested name")`, + "/Users/user/project/node_modules/@scope/ui/sub/unlisted.js": `console.log("TEST FAILED")`, + "/Users/user/project/node_modules/@scope/ui/package.json": /* json */ ` + { + "name": "@scope/ui", + "version": "1.0.0", + "main": "index.js", + "sideEffects": ["*.side.js", "polyfill.js"] + } + `, + }, + dce: true, + run: { + stdout: "root glob\nnested glob\nnested name\ndone", + }, + }); + // The list of the project itself, for files that the project imports by relative path. + itBundled("dce/PackageJsonSideEffectsNoSlashInProjectPackage", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "./deep/nested.side.js"; + import "./deep/polyfill.js"; + import "./deep/unlisted.js"; + console.log("done"); + `, + "/Users/user/project/src/deep/nested.side.js": `console.log("nested glob")`, + "/Users/user/project/src/deep/polyfill.js": `console.log("nested name")`, + "/Users/user/project/src/deep/unlisted.js": `console.log("TEST FAILED")`, + "/Users/user/project/package.json": /* json */ ` + { + "name": "app", + "sideEffects": ["*.side.js", "polyfill.js"] + } + `, + }, + dce: true, + run: { + stdout: "nested glob\nnested name\ndone", + }, + }); + // An entry without "/" has no directory, so it also lists a file that "main" reaches outside the package. + itBundled("dce/PackageJsonSideEffectsNoSlashOutsidePackage", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "by-name"; + import "by-glob"; + console.log("done"); + `, + "/Users/user/project/node_modules/shared/x.js": `console.log("name outside")`, + "/Users/user/project/node_modules/shared/y.js": `console.log("glob outside")`, + "/Users/user/project/node_modules/by-name/package.json": /* json */ ` + { + "name": "by-name", + "main": "../shared/x.js", + "sideEffects": ["x.js"] + } + `, + "/Users/user/project/node_modules/by-glob/package.json": /* json */ ` + { + "name": "by-glob", + "main": "../shared/y.js", + "sideEffects": ["y.*"] + } + `, + }, + run: { + stdout: "name outside\nglob outside\ndone", + }, + }); + // A file that the list names keeps its side effects when an onLoad plugin turns it into JavaScript. + itBundled("dce/PackageJsonSideEffectsListedFileLoadedByPlugin", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import "demo-pkg/root.scss"; + import "demo-pkg/scss/theme.scss"; + import "demo-pkg/scss/unlisted.less"; + console.log("done"); + `, + "/Users/user/project/node_modules/demo-pkg/root.scss": `.root { color: red }`, + "/Users/user/project/node_modules/demo-pkg/scss/theme.scss": `.theme { color: blue }`, + "/Users/user/project/node_modules/demo-pkg/scss/unlisted.less": `TEST FAILED`, + "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` + { + "sideEffects": ["./lib/common.js", "*.css", "*.scss"] + } + `, + }, + plugins(builder) { + builder.onLoad({ filter: /\.(scss|less)$/ }, async args => ({ + contents: `console.log(${JSON.stringify((await Bun.file(args.path).text()).trim())})`, + loader: "js", + })); + }, + run: { + stdout: ".root { color: red }\n.theme { color: blue }\ndone", + }, + }); + // Today a list with a glob is asked a second time through the nearest package.json that has a name, and that + // answer wins. A list of file names has no glob, so its own answer stands. This fails if a file name is a glob. itBundled("dce/PackageJsonSideEffectsFileNameInPackageWithoutName", { files: { "/Users/user/project/src/entry.js": /* js */ ` @@ -1042,22 +1155,28 @@ describe("bundler", () => { stdout: "root name\nroot glob\ncomp path\ndone", }, }); - // "\\" separates directories in an entry on Windows only. It is not part of a file name on any platform. + // An entry with "\\" is a path, as one with "/" is. With glob syntax its "\\" becomes "/" on every platform. + // Without glob syntax it is joined as written: a directory separator on Windows, a part of the name elsewhere. itBundled("dce/PackageJsonSideEffectsEntryWithBackslashIsAPath", { files: { "/Users/user/project/src/entry.js": /* js */ ` import "demo-pkg/sub/effect.js"; + import "demo-pkg/globbed/x.js"; + import "demo-pkg/deep/globbed/x.js"; console.log("done"); `, "/Users/user/project/node_modules/demo-pkg/sub/effect.js": `console.log("sub effect")`, + "/Users/user/project/node_modules/demo-pkg/globbed/x.js": `console.log("globbed x")`, + "/Users/user/project/node_modules/demo-pkg/deep/globbed/x.js": `console.log("TEST FAILED")`, "/Users/user/project/node_modules/demo-pkg/package.json": /* json */ ` { - "sideEffects": ["sub\\\\effect.js"] + "sideEffects": ["sub\\\\effect.js", "globbed\\\\*.js"] } `, }, + dce: true, run: { - stdout: isWindows ? "sub effect\ndone" : "done", + stdout: isWindows ? "sub effect\nglobbed x\ndone" : "globbed x\ndone", }, }); itBundled("dce/PackageJsonSideEffectsExactListInPackageWithoutName", { @@ -1086,8 +1205,9 @@ describe("bundler", () => { stdout: "listed\ndone", }, }); - // A list that has a glob does not store its ".css" entries. A list without a glob stores them. The CSS is in - // the bundle either way. A lazily loaded importer keeps an empty initializer for a CSS file with side effects. + // A list that has a glob does not store its ".css" entries, and a list without a glob stores them. This pins + // that difference as it is. The CSS is in the bundle either way. A lazily loaded importer keeps an empty + // initializer for a CSS file with side effects. itBundled("dce/PackageJsonSideEffectsCSSEntries", { files: { "/Users/user/project/src/entry.js": /* js */ ` @@ -1139,45 +1259,6 @@ describe("bundler", () => { expect(css).toContain(".exact"); }, }); - // The resolver asks the "sideEffects" list about every file that it resolves in the package. A file name in the - // list costs that ask one hash lookup, as a path does. A glob costs it one match: spelled "**/m0.js", these - // 20,000 names take 10.8 s of CPU on a debug build, against 0.2 s. - test("dce/PackageJsonSideEffectsFileNamesCostOneLookup", async () => { - const files = isDebug || isASAN ? 20 : 400; - const pkg = (name: string, spell: (i: number) => string) => ({ - "package.json": JSON.stringify({ - name, - version: "1.0.0", - main: "index.js", - sideEffects: Array.from({ length: 20_000 }, (_, i) => spell(i)), - }), - "index.js": - Array.from({ length: files }, (_, i) => `import "./unlisted${i}.js";\n`).join("") + "export const x = 1;\n", - ...Object.fromEntries(Array.from({ length: files }, (_, i) => [`unlisted${i}.js`, `globalThis.x${i} = 1;\n`])), - }); - using dir = tempDir("side-effects-file-names", { - "names.js": `import { x } from "names"; console.log(x);`, - "paths.js": `import { x } from "paths"; console.log(x);`, - node_modules: { - names: pkg("names", i => `m${i}.js`), - paths: pkg("paths", i => `./m${i}.js`), - }, - }); - const userCpu = async (entry: string) => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "build", entry, "--outdir=out"], - cwd: String(dir), - env: bunEnv, - stdout: "ignore", - stderr: "inherit", - }); - expect(await proc.exited).toBe(0); - return Number(proc.resourceUsage()!.cpuTime.user) / 1000; - }; - const paths = await userCpu("paths.js"); - const names = await userCpu("names.js"); - expect(names).toBeLessThan(paths * 4 + 100); - }); itBundled("dce/PackageJsonSideEffectsGlobInvalidPattern", { files: { "/Users/user/project/src/entry.js": /* js */ ` From e584e09d268a5bd26a6cfba0eb30a4cd9777e2d9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 24 Oct 2026 07:13:39 +0000 Subject: [PATCH 09/10] resolver: inline both halves of the sideEffects lookup The exact probe and the glob loop are inlined into each arm of `has_side_effects`, as they were before this branch. The probe for a file name stays a call. Release build, instructions for one lookup of a file that the list does not name: no field 28 on main and 29 here, a list of paths 140 and 149, a glob with "/" 1,411 and 1,407. --- src/resolver/package_json.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/resolver/package_json.rs b/src/resolver/package_json.rs index 9b412c306d9d..86d9827ef30c 100644 --- a/src/resolver/package_json.rs +++ b/src/resolver/package_json.rs @@ -255,10 +255,18 @@ pub struct MixedPatterns { } impl ExactEntries { - #[inline] + #[inline(always)] fn contains(&self, path: &[u8]) -> bool { - let has = |key: &[u8]| self.keys.contains_key(&StringHashMapUnownedKey::init(key)); - (self.has_paths && has(path)) || (self.has_names && has(bun_paths::basename(path))) + (self.has_paths && self.has_key(path)) || (self.has_names && self.has_name_of(path)) + } + + #[inline(always)] + fn has_key(&self, key: &[u8]) -> bool { + self.keys.contains_key(&StringHashMapUnownedKey::init(key)) + } + + fn has_name_of(&self, path: &[u8]) -> bool { + self.has_key(bun_paths::basename(path)) } } @@ -266,6 +274,7 @@ fn is_glob(entry: &[u8]) -> bool { strings::contains_any(entry, b"*?[{") || glob::has_extglob_prefix(entry) } +#[inline(always)] fn any_glob_matches(globs: &[glob::Prepared>], path: &[u8]) -> bool { if globs.is_empty() { return false; @@ -274,9 +283,12 @@ fn any_glob_matches(globs: &[glob::Prepared>], path: &[u8]) -> bool { let Ok(normalized_path) = PackageJSON::normalize_path_for_glob(path) else { return true; }; - globs - .iter() - .any(|pattern| pattern.r#match(&normalized_path).matches()) + for pattern in globs { + if pattern.r#match(&normalized_path).matches() { + return true; + } + } + false } impl SideEffects { From af8b80f8c57ceed2c7003bce9d156b846060bcd3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 24 Oct 2026 08:18:42 +0000 Subject: [PATCH 10/10] ci: retrigger