Repository navigation
bundler: stop a sideEffects array from enabling the barrel optimization - #40651
Conversation
…: false in the barrel optimization The resolver gave a file that does not match a sideEffects array the same verdict (NoSideEffectsPackageJson) as a file in a sideEffects: false package. The barrel optimization used that verdict to defer unused re-exports before resolution, so a re-export target that the array lists was never loaded. Add a separate verdict, NoSideEffectsPackageJsonArray, for the array form. The linker still tree-shakes such files, but the barrel optimization no longer defers their re-exports. Fixes #40650
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review. WalkthroughThe change distinguishes package-wide ChangesPackage side-effects handling
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the problem, fix, expected behavior, background, and verification results. It does not use the template headings exactly, but it provides the required information in equivalent sections. Comment |
|
Updated 6:28 AM PT - Aug 27th, 2026
❌ @robobun, your commit 83ef366 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 40651That installs a local version of the PR into your bun-40651 --bun |
There was a problem hiding this comment.
LGTM — targeted fix that correctly narrows barrel deferral to the "sideEffects": false case only.
What was reviewed:
- Audited every consumer of
ast::SideEffects: the barrel gate atbarrel_imports.rs:119compares== NoSideEffectsPackageJson(now excludes arrays), whileLinkerContext.rs:360andbundle_v2.rs:6606compare!= HasSideEffects, so tree-shaking of unlisted files is preserved. No exhaustivematchon this enum exists to miss the new variant. - Verified
primary_side_effects:Unspecifiedreturnstruefromhas_side_effects()and hitsHasSideEffects, so the newelsearm is only reached forMap/Glob/Mixed— theArrayname is accurate. - Tests cover exact entry, glob entry, and the negative (unlisted still dropped), placed in the existing
bundler_barrel.test.tsviaitBundled.
Extended reasoning...
Overview
This PR fixes issue #40650: the bundler's barrel optimization was incorrectly deferring re-export resolution for packages with a "sideEffects" array, treating it identically to "sideEffects": false. The fix adds a new SideEffects::NoSideEffectsPackageJsonArray variant in src/ast/loader.rs and has primary_side_effects() in src/resolver/resolver.rs return it for the Map/Glob/Mixed package.json forms. Since apply_barrel_optimization gates on == NoSideEffectsPackageJson specifically, array-form packages no longer trigger deferral, so re-export targets are resolved and their own array match runs.
Security risks
None. This is a bundler tree-shaking correctness fix touching an internal enum and a three-arm helper. No parsing of untrusted input changes, no auth/crypto/permissions paths, no FFI or memory-owning code.
Level of scrutiny
Low-to-moderate. The diff is ~10 lines of Rust plus tests. The main risk with adding an enum variant is missing a consumer that needed updating; I grepped every reference to SideEffects:: in src/ and confirmed there are no exhaustive matches on this enum — every consumer uses ==/!= comparisons, and the two != HasSideEffects sites correctly treat the new variant the same as the old one for tree-shaking purposes. I also confirmed package_json::SideEffects::Unspecified cannot reach the new else arm because has_side_effects() returns true for it.
Other factors
Test coverage is solid and follows repo conventions: three itBundled cases in the existing test/bundler/bundler_barrel.test.ts covering exact array entry, glob array entry, and the negative case (unlisted module still shaken). The trade-off — array-form packages lose the barrel-deferral perf optimization entirely — is the conservative-correct choice since deferral happens before resolution and cannot know whether a target is listed. No CODEOWNERS cover these paths. Exit reason was dry_streak.
|
CI note: the three red tests in build 106866 are unrelated to this diff. macro-test.test.ts (x64-asan) and url.test.ts (macOS x64) also fail on main. require-cache.test.ts is a Windows RSS-threshold test that this change does not touch. All bundler suites, including the new tests, pass on every lane. |
…rred (#40664) ### Problem - `Bun.build({ minify: true })` is not deterministic. The same tree produces two outputs of identical byte length whose generated identifier names differ. Reproduced with the issue's repo: under CPU load, 2 to 4 of 60 in-process builds flip between the two reported hashes (`706328dd39c9a25` and `b667ca6974bb6b5d`). - The cause is in the barrel optimization (`src/bundler/barrel_imports.rs`). When a request un-defers a deferred barrel import record, the BFS propagates only the one alias that triggered it (`schedule_barrel_deferred_imports`). A record that is live when the barrel finishes parsing instead gets every alias seeded by Phase 1. So the alias set that reaches a downstream barrel depends on parse completion order. - In the slow order the downstream barrel keeps records deferred (`IS_UNUSED`). `advance_import_tracker` (`src/bundler/LinkerContext.rs:3380`) then treats the re-export as external, the symbol merge is skipped, and the minify renamer sorts a different symbol set into the name slots. ### Fix - When `un_defer_record` flips a record from deferred to live, push a BFS item for every alias the barrel imports through that record, not just the requested one. This is the same set Phase 1 seeds for a live record, so both orders converge to one deferral state. - The emitted module set does not change. The propagated aliases go through the existing dedup in the BFS. - Verified: `test/bundler/bundler_barrel.test.ts` (new test, stock bun fails it on every run). All 58 tests in the file pass. The issue's repro repo is byte-stable for 100 builds under CPU contention with the fix, and flips 2 to 4 times in 60 without it. ### Background - The barrel optimization defers the import records of a pure re-export file (`sideEffects: false`) until some file requests one of its exports. `requested_exports` records requests eagerly, and a BFS un-defers records through chains of barrels as requests arrive. - Deferral state is decided on the bundle thread in parse completion order, which varies with load. The compensation logic must therefore converge to one final state for every order. The single-alias propagation was the one path that did not. - The test pins the order with a 4 MB comment: it slows the parse of one consumer without changing minified output, and builds the same graph in both orders. The outputs must be byte-identical. <details><summary>Notes</summary> - Instrumented the minify renamer inputs and diffed a flapped run against a normal one: char frequency and use counts were identical, but symbols such as `isErrorRetry` and `SCHEMA_ERROR_CODE_TITLES` resolved to the re-exporting barrel's import items in the flapped run instead of the defining files. The `symbols.merge` calls for the barrel's items were missing entirely. - A link-time dump of `IS_UNUSED` records showed exactly two records of `packages/core/src/exports/error-contract.ts` left deferred and unresolved in flapped runs. Both target modules were in the bundle through direct imports elsewhere, which is why the output length and string literals stay identical and only names move. - Event logs of the barrel BFS showed the divergence: when `core/src/index.ts` parsed before its first consumer, its own record to `error-contract` was deferred, and the later un-defer propagated only the handful of aliases consumers had requested by then. When it parsed after a consumer, Phase 1 seeded all 40 aliases of the record. - The issue's repro depends on a `sideEffects` array being read as `false` (#40650, fix in #40651). That fix would hide this repro but not the order dependence for real `sideEffects: false` packages, which this PR fixes. - Ran the new test 5 times under the fixed debug build (5 of 5 pass) and 4 times under stock bun 1.4.1 (4 of 4 fail), so the order pinning is stable in both directions. </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 0 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/bundler_barrel.test.ts bun test v1.4.1 (731aa92) test/bundler/bundler_barrel.test.ts: (pass) bundler > barrel/SkipUnusedWithOptimizeImports [573.27ms] (pass) bundler > barrel/AllExportsNeeded [118.46ms] (pass) bundler > barrel/SkipUnusedWithSideEffectsFalse [105.32ms] (pass) bundler > barrel/NoOptimizationWithoutSideEffects [120.59ms] (pass) bundler > barrel/ExportStarLoadsAll [85.68ms] (pass) bundler > barrel/NonBarrelWithLocalExports [95.90ms] (pass) bundler > barrel/NamespaceImportLoadsAll [99.78ms] (pass) bundler > barrel/OutputEquivalence [98.89ms] (pass) bundler > barrel/DefaultReExport [111.83ms] (pass) bundler > barrel/ImportThenExport [93.75ms] (pass) bundler > barrel/ReExportChain [103.64ms] (pass) bundler > barrel/StarWithNamedFromSameSource [117.43ms] (pass) bundler > barrel/SideEffectOnlyImport [123.39ms] (pass) bundler > barrel/MultipleImporters [172.56ms] (pass) bundler > barrel/CircularExports [382.28ms] (pass) bundler > barrel/CircularStarExports [370.30ms] (pass) bundler > barrel/NamespaceReExportCycle ... (truncated) release without fix: 3 FAILED bun test v1.4.1-canary.1 (731aa92) test/bundler/bundler_barrel.test.ts: (pass) bundler > barrel/SkipUnusedWithOptimizeImports [12.85ms] (pass) bundler > barrel/AllExportsNeeded [3.43ms] (pass) bundler > barrel/SkipUnusedWithSideEffectsFalse [2.87ms] (pass) bundler > barrel/NoOptimizationWithoutSideEffects [2.73ms] (pass) bundler > barrel/ExportStarLoadsAll [2.37ms] (pass) bundler > barrel/NonBarrelWithLocalExports [2.18ms] (pass) bundler > barrel/NamespaceImportLoadsAll [2.18ms] (pass) bundler > barrel/OutputEquivalence [3.20ms] (pass) bundler > barrel/DefaultReExport [2.80ms] (pass) bundler > barrel/ImportThenExport [2.70ms] (pass) bundler > barrel/ReExportChain [3.14ms] (pass) bundler > barrel/StarWithNamedFromSameSource [2.29ms] (pass) bundler > barrel/SideEffectOnlyImport [3.22ms] (pass) bundler > barrel/MultipleImporters [3.39ms] (pass) bundler > barrel/CircularExports [10.07ms] (pass) bundler > barrel/CircularStarExports [9.04ms] (pass) bundler > barrel/NamespaceReExportCycleThroughStarTarget [9.91ms] (pass) bundler > barrel/SelfReExport [4.12ms] (pass) bundler > barrel/DynamicImportInSubmodule [4.08ms] (pass) bundler > barrel/DynamicImportWithStaticImportS ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/bundler_barrel.test.ts bun test v1.4.1 (731aa92) test/bundler/bundler_barrel.test.ts: (pass) bundler > barrel/SkipUnusedWithOptimizeImports [554.95ms] (pass) bundler > barrel/AllExportsNeeded [113.28ms] (pass) bundler > barrel/SkipUnusedWithSideEffectsFalse [103.53ms] (pass) bundler > barrel/NoOptimizationWithoutSideEffects [114.07ms] (pass) bundler > barrel/ExportStarLoadsAll [101.98ms] (pass) bundler > barrel/NonBarrelWithLocalExports [101.90ms] (pass) bundler > barrel/NamespaceImportLoadsAll [102.32ms] (pass) bundler > barrel/OutputEquivalence [111.37ms] (pass) bundler > barrel/DefaultReExport [120.43ms] (pass) bundler > barrel/ImportThenExport [104.06ms] (pass) bundler > barrel/ReExportChain [113.12ms] (pass) bundler > barrel/StarWithNamedFromSameSource [110.32ms] (pass) bundler > barrel/SideEffectOnlyImport [103.36ms] (pass) bundler > barrel/MultipleImporters [120.47ms] (pass) bundler > barrel/CircularExports [362.79ms] (pass) bundler > barrel/CircularStarExports [355.58ms] (pass) bundler > barrel/NamespaceReExport ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision 842a7df features baseline 23 deps, 129 codegen, 1172 objects in 678ms ninja: Entering directory `/workspace/bun/build/release' [1/1244] install /workspace/bun bun install v1.4.1-canary.1 (731aa92) Checked 26 installs across 63 packages (no changes) [8.00ms] [2/1244] gen ErrorCode+*.h [3/1244] gen bindgenv2 [4/1244] install /workspace/bun/packages/bun-error bun install v1.4.1-canary.1 (731aa92) Checked 1 install across 2 packages (no changes) [1.00ms] [5/1244] install /workspace/bun/src/node-fallbacks bun install v1.4.1-canary.1 (731aa92) Checked 111 installs across 104 packages (no changes) [6.00ms] [6/1244] fetch tinycc [tinycc] up to date [7/1243] gen node-fallbacks/react-refresh.js Bundled 1 module in 7ms react-refresh.js 4.81 KB (entry point) [8/1243] gen bake.{client,server,error}.js -> bake.client.js, bake.server.js, bake.error.js [9/1243] fetch libjpeg-turbo [libjpeg-turbo] up to date [10/1216] gen ProcessBindingConstants.lut.h Generating /workspace/bun/ ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/bundler/barrel_imports.rs | 33 ++++++++++++++- test/bundler/bundler_barrel.test.ts | 81 ++++++++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 2 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/bundler/barrel_imports.rs 1 4 0 test/bundler/bundler_barrel.test.ts 2 2 0 ``` </details> **root cause** · written by the author bot The bundler's barrel-file optimization deferred re-export records in parse-completion order, and when a deferred record was later revived by a downstream request, only the single requested alias was propagated instead of all aliases the barrel imported through that record. Under load, a different parse-completion order could leave the remaining aliases permanently marked unused, causing advance_import_tracker to treat them as external and sever symbol merges, which shuffled minified identifier names between otherwise identical builds. The fix propagates every alias through a record when it … <!-- robobun:evidence:end -->
Problem
sideEffectsarray behaves likesideEffects: false. A module the array names is tree-shaken away when it is only reachable through a pure re-export barrel. Reported in bundler: asideEffectsarray is treated assideEffects: false— a module the array names is still shaken away #40650.SideEffects::NoSideEffectsPackageJson, as a file in asideEffects: falsepackage (src/resolver/resolver.rs:6747).apply_barrel_optimization(src/bundler/barrel_imports.rs:119) reads that verdict and defers the barrel's unused re-export records before resolution, so a listed target is never loaded and its own array match never runs.Fix
SideEffects::NoSideEffectsPackageJsonArray, for a file that does not match asideEffectsarray. The resolver returns it instead ofNoSideEffectsPackageJsonfor the array form.NoSideEffectsPackageJsononly, so an array no longer enables deferral. The barrel's re-exports are resolved, and a listed target getsHasSideEffectsfrom its own array match.HasSideEffects, so the linker still tree-shakes an unlisted file. The unlisted-module case keeps its behavior.test/bundler/bundler_barrel.test.ts(exact entry, glob entry, unlisted entry). Stock bun fails the first two. Also ran all ofbundler_barrel.test.ts,esbuild/dce.test.ts,bundler_edgecase.test.ts, andtest/bake/dev/bundle.test.ts.Background
sideEffects: false, or an explicitoptimizeImportsentry. With an array, a deferred target can itself be listed, and that can only be seen after resolution.[review] gate passed · iteration 0 · 3 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file
root cause · written by the author bot
The resolver returned the same verdict for a file in a package with
sideEffects: falseand a file that simply did not match a package'ssideEffectsarray, so the barrel optimization treated any array-form package as fully pure and deferred unused re-export records before the named module could be resolved and evaluated. The fix introduces a distinct verdict,NoSideEffectsPackageJsonArray, which the resolver returns for the array form, and the barrel check only accepts the package-widesideEffects: falseverdict, so array-form packages no longer trigger deferral. All other consumers …