Repository navigation
bundler: give each entry point its own output and module order without --splitting - #41263
Merged
Merged
Conversation
Without code splitting, compute_chunks keyed each entry point's chunk by the full entry_bits of its file. Entry points that reach each other have the same entry_bits, so they shared one chunk and only the last one was written. CSS entry points hashed the same bits and had the same problem. Key both by the entry point's own bit, as code splitting already does. Each non-split chunk also visits its own entry point first when it orders its files. Distances are the minimum over all entry points, so another entry point in the chunk could sort first and run its imports too early.
In a debug build, --compile inlines process.versions.bun as "<version>-debug". compile/HelloWorldWithProcessVersionsBun removed "-debug" only from the expected value, so it failed on every debug build. The API variant of the test already removes it from both values.
…oint Without code splitting, the file of one entry point can print in the chunk of another entry point. In cjs output, that file assigned its own exports to module.exports and replaced the exports of the chunk. Its re-exports from other modules also went onto module.exports. Both writes now happen only when the file is the entry point of the chunk, as in esbuild. A file that is not an entry point also stops writing its re-exports to module.exports. importstar/ReExportStarEntryPointAndInnerFileExternal gets back the value that esbuild prints. The value changed when the test first ran with cjs output. Two entry points can name one file, for example when an onResolve plugin maps two names to it. LinkerGraph::load now keeps one entry point for the file, so the file gets one chunk and one output file.
Collaborator
Author
|
Status Reproduced on 1.4.1 (canary a6c4cc2) and on main, Linux x64:
This PR fixes all three. The new tests fail on 1.4.1 and pass with the change. CI runs on the latest push. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. WalkthroughChangesThe bundler now deduplicates entry files, creates entry-specific chunk keys, orders entry-point traversal consistently, and limits CommonJS namespace exports to the correct entry file. Regression tests cover JavaScript, CSS, HTML, plugin, compiled, and CommonJS scenarios. Entry-point chunk handling
Possibly related PRs
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change corrects separate entry-point bundling and CommonJS wrapper initialization while adding targeted regression coverage. No current merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description provides detailed problem, fix, background, verification, test coverage, and related context. It does not use the template headings exactly, but it covers the required information under equivalent sections. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes core linker semantics — entry-chunk keying, module walk order, and which files write module.exports in cjs output (including in single-entry builds) — a human look is still worthwhile.
What was reviewed:
- Chunk keying in computeChunks.rs: single-bit key is unique per entry after the LinkerGraph dedupe, and the dropped HTML prefix byte only affected sort order between entry chunks, not chunk identity.
- The rotate_right in findAllImportedPartsInJSOrder preserves the stable sort of the rest of the distance-0 group and is a no-op when the entry point already sorts first.
- The split_last drop in generateCodeForFileInChunkJS is gated on
force_include_exports_for_entry_point, which is the same flag doStep5 uses to append the statement, so it can't strip a user statement. - The importstar/ReExportStarEntryPointAndInnerFileExternal expectation change matches the value esbuild produces and matches the sibling non-external test.
Extended reasoning...
Overview
This PR fixes a bundler bug where, without --splitting, entry points that import each other collapse into one output file (the second overwrites the first because both share the same entry_bits key). The fix touches six linker files: computeChunks.rs now keys every entry chunk by its own single bit unconditionally (dropping the non-splitting branch and its HTML-disambiguation prefix byte); LinkerGraph.rs deduplicates user-specified entry points that resolve to the same source and replaces an unsafe set_len + column-write pattern with append_assume_capacity; findAllImportedPartsInJSOrder.rs rotates the chunk's own entry point to the front of the distance-0 sort so the import walk starts there; convertStmtsForChunk.rs and generateCodeForFileInChunkJS.rs restrict the two linker-inserted module.exports writes to the file that is this chunk's entry point; Chunk.rs adds an is_entry_point_file helper. Nine new bundler tests cover ESM/CJS/CSS/HTML/--compile/plugin variants, plus one existing importstar expectation is changed and an unrelated -debug version-string test is fixed.
Security risks
None. This is bundler output-shaping logic — chunk grouping, statement ordering, and which file writes module.exports. No untrusted input parsing, no auth/crypto/network, no filesystem path handling beyond what the bundler already does.
Level of scrutiny
High. The change is well-reasoned and thoroughly tested, but it alters user-visible bundler output in several ways at once: which files are emitted, the module execution order within a chunk, what require() of a cjs bundle returns (the __reExport(..., module.exports) change also affects single-entry builds, as the PR notes), and the sort order of entry chunks in the output list (the dropped HTML-first prefix byte). It also changes an existing test's expected value. Each of these is defended in the description with reference to esbuild behavior, but the interactions are subtle enough — e.g., the walk-order change and the two cjs-write changes must land together for correctness — that a maintainer familiar with the linker should confirm the design.
Other factors
The bug hunt ran to a dry streak with no findings. I checked that the debug_assert!(!js_chunk_entry.found_existing) cannot fire now that LinkerGraph::load skips duplicate source indices, that entry_bits (the full set) is still cloned and stored on the chunk for the later per-file assignment loop, and that the split_last drop is guarded by exactly the flag that controls whether step 5 appended the statement. The LinkerGraph.rs refactor is a net safety improvement (no more set_len on uninitialized columns). No CODEOWNERS entry covers src/bundler/. The unrelated bundler_compile.test.ts version-string fix is small and correct.
… exists A file that something require()s gets an ESM wrapper. When all of its top-level statements hoist, the parser gives the file no wrapper symbol, and the printer prints the file with no wrapper. In cjs output, the tail of that entry point still called the missing wrapper. It printed `__INVALID__REF__();`, so loading the output threw a ReferenceError. The esm tail already checks that the symbol exists.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
+278
to
+279
| /// Whether `source_index` is the entry point of this chunk. Without code | ||
| /// splitting, the files of other entry points can print in this chunk too. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+694
to
+695
| // Two entry points can name one file, e.g. when an onResolve plugin | ||
| // maps two names to it. The file gets one entry point and one output. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+111
to
+114
| // Entry point chunks are keyed by ONLY the entry point's own bit, not the full | ||
| // entry_bits. Entry points that reach each other have the same entry_bits, and | ||
| // each one still needs its own chunk. With code splitting, an entry point file | ||
| // that other entry points reach (e.g., via re-exports) goes into a shared chunk. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+65
to
+66
| // If this file is the CommonJS entry point of this chunk, double-write re-exports | ||
| // to the external CommonJS "module.exports" object in addition to our internal ESM |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+761
to
+762
| // This stays the last statement of the part: `generate_code_for_file_in_chunk_js` | ||
| // drops it when another entry point's chunk prints this file. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+109
to
+112
| // Without code splitting, a chunk holds every file that its entry point reaches. | ||
| // Distances are the minimum over all entry points, so other entry points in this | ||
| // chunk also sort at distance 0. Visit this chunk's own entry point first, so the | ||
| // files print in the order that this entry point imports them. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+291
to
+292
| // Step 5 ends this part of a CommonJS entry point with | ||
| // `module.exports = __toCommonJS(exports)`. Only the chunk of that entry point runs it. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+1227
to
+1228
| // A file without a wrapper symbol prints unwrapped (`needs_wrapper_ref` | ||
| // in the parser), so there is no `init_foo` to call. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
This was referenced Sep 6, 2026
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 12, 2026
#41550) ### Problem - `test/bundler/bundler_compile.test.ts` reports `timeout on :windows: 11 aarch64` (builds 110724, 110766, 110870). The output stops after a different test each time and no per-test timeout fires. The file is not hung. It takes 286 to 293 s in the passing runs and the runner caps it at 300 s (`integrationTimeout`, `scripts/runner.node.mjs:90`). - The cost is Smart App Control. Windows 11 ships it in evaluation mode. On the first launch of an unsigned executable it hashes the file and asks the cloud for its reputation. A 77 MB `bun build --compile` output pays 2.3 s for that on the aarch64 VM, more in canadaeast and switzerlandnorth. The file launches about 85 such binaries. - `scripts/bootstrap.ps1` tries to disable Defender, but tamper protection blocks `DisableRealtimeMonitoring` on Windows 11, `Uninstall-WindowsFeature` does not exist there, and the `C:\` exclusion does not cover Smart App Control. ### Fix - `scripts/runner.node.mjs`: at job start on Windows, set `HKLM\SYSTEM\CurrentControlSet\Control\CI\Policy\VerifiedAndReputablePolicyState` to 0 and run `CiTool --refresh`. This takes effect at once, with no reboot. It runs next to the existing `Set-DnsClientServerAddress` step, which already needs the same rights. - `scripts/bootstrap.ps1`: `Disable-Smart-App-Control` in `Optimize-System`, so the next image bake has it off from the start. No `# Version` bump and no `[publish images]`: the runner step covers the current v23 images. - Verified on a Windows 11 aarch64 VM from the CI image: the file takes 205 s with the policy in evaluation mode and 28 s with it off (85 pass both times). The runner step logs `Smart App Control: state 2 -> 0` and is a no-op once the value is 0. ### Background - Smart App Control is a Windows 11 code integrity policy. Evaluation mode is the default on a fresh install. It does not block anything, but it still evaluates every new unsigned binary. Once turned off it cannot be turned on again without a reinstall. - The runner has two per-file caps: 3 min, and 5 min for files matched by the integration list (`bundler_compile` is on it). A file that passes the cap is reported as `timeout` and retried once. - Only the Windows 11 aarch64 lane is affected. The Windows 2019 x64 lane has Defender uninstalled and has no Smart App Control. <details><summary>Notes</summary> There is no single culprit commit. The file has grown to 85 tests (#41263 and #40816 added the latest ones) and the per-launch cost depends on the Azure region the spot VM lands in. The three failures ran in canadaeast and switzerlandnorth. How the cause was found: - The runner log for the three failures ends after a different compile test each time (the "8+ entry points" test in 110724, the last test in 110766 and 110870), followed by `EPERM` from the runner's `rmSync` of the temp dir: the killed test's child was still running. No `bun test` per-test timeout fired, so nothing was stuck, the file was cut off mid-run. - Shard 1 of the Windows 11 aarch64 lane, durations of the file in passing builds (job log "Ran 85 tests across 1 file"): 219 to 226 s on eastus2 and swedencentral VMs, 286 to 293 s on canadaeast VMs. The NAT gateway IP of the agent maps to the region. - On the dev VM (same image lineage, Windows 11 26100, buildkite-agent service present): first launch of a fresh compiled exe 2300 ms, second launch 17 ms. A copy of the same file pays the 2300 ms again. A 50 KB `where.exe` with 77 MB of random bytes appended (not mapped by the loader) pays 2600 ms. A copy of the signed `node.exe` (89 MB) pays 800 ms. Defender exclusions for `C:\` and `D:\` were already in place. - `Get-MpComputerStatus` shows `SmartAppControlState: Eval` and the Code Integrity log lists the `VerifiedAndReputableDesktopEvaluation` policy as active. After `VerifiedAndReputablePolicyState = 0` and `CiTool --refresh`: state `Off`, first launch 154 ms, full file 28 s. Suites run on the dev VM after the change: `bundler_compile.test.ts` (85 pass), `bundler_compile_autoload.test.ts`, `bundler_compile_splitting.test.ts`, `transpiler/property.test.ts` through `runner.node.mjs`. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · build/CI scripts only; test-proof not applicable <!-- robobun:evidence:end -->
Jarred-Sumner
added a commit
that referenced
this pull request
Sep 26, 2026
…r evaluates it (#42695) Fixes #42632. Supersedes #41386 (its four tests are included). ### Problem - With `--splitting`, a chunk that several entry points share can print an import cycle importer-first. A hoisted `var` then reads as `undefined`: `TypeError: undefined is not an object (evaluating 'a.name')` in opencode built by bun 1.4.1+. - `find_imported_parts_in_js_order` walked each chunk from its files sorted by `(distance_from_entry_point, stable_source_index)`. Neither key tracks evaluation order, so a walk could enter a cycle at the file that has to run first. That file prints last. ### Fix - `EntryWalk` is the traversal of the old visitor, started from an entry point. The walks run in parallel, one per entry point. With `--splitting`, a chunk has one owner, the entry point that loads first among its entry points (`LoadOrder`), and only the owner's walk places its files. Without `--splitting`, each chunk is owned by its own entry point. - The walk records runs of parts. The layout is one linear pass. A build with one entry point prints the same bytes as on main (92 packages). - Deletes `chunk::Order` and `File.distance_from_entry_point`. - Verified: `test/bundler/bundler_splitting.test.ts` (26 new tests, 6 fail on main). Notes: bytes of 92 packages, 6,000 random graphs, wall time. ### Background - A shared chunk holds files that several entry points load. - `entry_bits[file]` has one bit per entry point that loads the file. - A file runs after the files it imports. Inside an import cycle, the file entered first runs last. - A part is one top-level statement. An `import` of a wrapped file prints as a call in its part, so parts print between the files that the file imports. <details><summary>Notes</summary> **Shape of the change** - `EntryWalk` (`findAllImportedPartsInJSOrder.rs`) has the rules of the old `FindImportedPartsVisitor`: it follows every `import` statement of every part, also in files that tree shaking dropped, and the other records of parts that run. It differs in where it starts and in what it records. - There is one walk per entry point that owns a chunk, and the walks run on the worker pool (e702bec). With `--splitting`, the owner of a chunk is the entry point that loads first among the entry points of the chunk. Only the owner's walk places the files of the chunk, so no two walks write the same list. A walk goes through the files of other chunks. Without `--splitting`, each chunk is owned by its own entry point (#41263: each entry point keeps its own module order). - `LoadOrder` ranks the entry points in one pass before the walks: the user's entry points, then the chunks that load on demand (`import()`, a `require()` in a part that only declares) in the order evaluation comes to them, then the rest. A split `require()` in a part that runs at load loads its chunk in place, and a walk follows it in place, like a static import. A load counts only the calls in files that its entry point loads. - A placement is a `PartRun`: parts `begin..end` of a file. The parts ahead of the part that imports a file go before that file, the rest goes where the walk leaves the file. A wrapped file goes whole, where the walk leaves it. `ChunkLayout::place` turns the runs of a chunk into part ranges in one linear pass. It has the membership checks and the range merging of the old visitor. - Every claim happens when the frame pops, not when it is pushed (the class-name object of a CSS file, the rank of a split `require()` target, the on-demand list). An import pushed earlier can reach the same target first. - The class-name object of a CSS module goes into the chunk of its importer when that chunk loads the CSS file, whatever load reached the importer. - A part also leads to the files that declare the bindings it uses (`part.dependencies`), for a file that the `import` statements did not reach. The namespace export part is the exception: it comes ahead of the `import` statements and only holds getters, so what it names is walked after the other parts. With CommonJS output it names every export of the entry point, and it must not decide the order. - `reached_chunks_in_order` is a walk per chunk from the chunk's files in the order the pass entered them. It ranks the other chunks for the chunk's `import` statements. It does not order files. - `mark_file_reachable_for_code_splitting` keeps the BFS from main. Only the distance is gone. - Chunk folding rewrites `entry_bits`. A folded file can sit in a chunk whose entry points do not import it. It goes last in its chunk, as it did with the sorted roots on main. **Why the order is not part of the entry bits walk** The previous revision of this PR (f10ab25) gave out positions in `mark_file_reachable_for_code_splitting`. CI showed two regressions, and both come from that merge: - A file's parts interleave with its imports. `import "./a.cjs"; import "./b.js"` prints `require_a()` in a part of the importer, ahead of the code of `b.js`. One position per file puts all parts of the importer after all of its imports: `b` ran before `a` (`react-compiler/BundledReactPreservesImportRefs`, the happy-dom hash in `bundler_bytecode_portable`). - The load graph is not the import graph. The entry bits walk follows what loads. An `import` of a `sideEffects: false` file is no edge there, the walk reaches the file from the part that uses the binding, and it never visits a barrel that tree shaking dropped. A DFS over those edges prints such a file at its first use: `import { x } from "pure"; import "./side.js"` ran `side` first. In a build with one entry point and no `--splitting`, 20 of 92 packages printed different bytes than on main. The order has to follow the import statements, and a walk may place a file only if its entry point loads it (the barrel and unused-import tests). Whether an entry point loads a file is known only when the bits are complete. So the order is its own pass, after the bits. **Shapes** (stdout of the bundle, `ok` = same as the source) | shape | main | #41386 | this PR | |---|---|---|---| | #42632: cycle, both files at the same import depth | `undefined` | ok | ok | | cycle, the file to run first is deeper | `undefined` | `undefined` | ok | | no cycle, import depth differs from evaluation order | `q p` | `q p` | `p q` (ok) | | lazy page listed before its importer enters the cycle from the other side | ok | `undefined` | ok | | earlier entry point reaches the cycle through a `sideEffects: false` barrel, or an unused import | ok | ok | ok | | top-level `require()` of a split chunk enters the cycle first (`--target=bun`) | ok | ok | ok | | two `import()` calls, the one in an imported file runs first | `undefined` | ok | ok | **Bytes against main** 92 packages from `test/node_modules`, one entry point each (the `module` or `main` file of the package). On the current head, against main c6b7fcb: - `--target=bun`, without and with `--splitting`: all 92 print the same bytes as main. `--format=cjs`: all 92 the same. - Two entry points without `--splitting` (44 pairs): all the same. - Two entry points with `--splitting` (44 pairs): 42 the same. `@grpc/grpc-js` + `@grpc/proto-loader` and `socket.io` + `socket.io-adapter` differ in the order of the lazy `__commonJS` wrappers inside the shared chunk. On e9f9214, against main 7000dc1, also `--bytecode`, `--target=node --format=cjs` and `--minify` (all 92 the same), and `--target=browser` with and without `--format=iife` (the 55 that build are the same). The webpack bundle differs from run to run on main itself, in how `require.resolve("watchpack")` prints. That is a separate bug and I did not count it. The first revision of the walk in this PR (f10ab25) differed from main on 20 of the 92 in the first mode. **Differential run** (measured on e9f9214, before e702bec moved the walks to the worker pool) The generator from `test/bundler/splitting-fuzz.ts` (#42442), 2,000 graphs per row, `--splitting --target=bun --format=esm`. The oracle is the unbundled source. A graph counts when a value that a module reads while it loads differs from the source run. | row | main | #41386 | this PR | |---|---|---|---| | entry points load their chunks with `import()` | 138 | 123 | 114 | | static entry points | 147 | 133 | 122 | | static entry points, no `require()` | 16 | 14 | 5 | | the same, graphs with one entry point (1,523) | 8 | 7 | 0 | Reads of a binding before its initializer ran: 71 / 55 / 44, 76 / 61 / 48, 12 / 12 / 4. Bundles that are byte for byte the same as on main: 1,933, 1,944 and 1,975 of 2,000. The graphs that stay wrong are wrong on main too (a `require()` cycle that the bundle cannot reproduce, a cycle that two chunks share), except four seeds where main is right and this PR is not: 146 and 1131 (two entry points need opposite orders), 655 and 1744 (below). **Wall time** (measured on e9f9214, where the pass over all entry points was serial) Release builds, main 7000dc1 against this PR, median wall time of `bun build`, runs interleaved. The machine had other load, so read differences under 5% as noise. | project | main | this PR | runs | |---|---|---|---| | 800 entry points x 13 modules over a 3,000-module core, `--splitting` | 776 ms | 754 ms | 11 | | the same, no `--splitting` | 1,987 ms | 1,942 ms | 9 | | 800 entry points x 13 modules over one 200,000-export file (#42489), `--splitting` | 1,336 ms | 1,296 ms | 9 | | 300 entry points over the 3,000-module core, `--splitting` | 259 ms | 251 ms | 11 | | the same, no `--splitting` | 722 ms | 708 ms | 15 | | one entry point, 200 routes behind `import()`, all over the 3,000-module core, `--splitting` | 276 ms | 266 ms | 11 | Peak RSS is the same within 1% in every row. **Prior art** - esbuild has the same sort key, from which bun's was ported (`linker.go` `findImportedPartsInJSOrder`), and the same bug: evanw/esbuild#399, open since 2020. - Rollup (`analyseModuleExecution`, `src/utils/executionOrder.ts`) and Rolldown (`sort_modules`) number every module in one post-order DFS from the entry points and sort each chunk by that number. This PR does the same with the walk that bun already had, and keeps the placement per part. **opencode** opencode v1.18.30, its `script/build.ts` options without `compile` (355 outputs). Bundle from main: `opencode run "say hi"` fails, the log has `TypeError: undefined is not an object (evaluating 'node.name')`. Bundle from this PR: the same command starts the session and reaches the call to the model API. **Remaining limits** - When two loaders evaluate the files of a shared chunk in opposite orders, the chunk follows the first. One printed order cannot serve both (seeds 146 and 1131). - A split `require()` counts as `runs at load` when its part has side effects. A function that holds the call and that its own file calls while it loads is on demand (seed 1744). A callback inside a top-level statement that never runs while loading counts as at load. - Seed 655: a top-level `require()` closes a cycle across two chunks. From source, `require()` returns the half-initialized module. In the bundle it runs the other chunk. - The class-name object of a CSS module prints ahead of the first file of the chunk that imports it. Main printed it where its walk first came to the file, also through a dropped file, and also in chunks that do not use it. **Suites** On a debug build of this commit: - `test/bundler/bundler_splitting.test.ts`: 151 pass on the current head. Main fails 6 of the 26 new tests: `TreeShakenImporterPreservesCyclicChunkOrder`, `CyclicChunkPreservesDeepImporterOrderSplit`, `SharedChunkKeepsImportCycleOrder`, `SharedChunkCycleOrderIgnoresImportDepth`, `SharedChunkKeepsSideEffectOrder`, `SharedChunkFollowsTheImportCallThatRunsFirst`. The other new tests guard the walk: earlier revisions of this PR fail `WrappedImportKeepsImportOrder`, `SideEffectsFalseFileKeepsImportOrder`, `SharedChunkKeepsWrappedImportOrder` `NamespaceExportKeepsImportOrder`, `SharedChunkIgnoresImportCallInFileThatEntryDoesNotLoad` and `CssModuleObjectOfFileReachedThroughSplitRequire`. - Pass (on e9f9214): the 13 files of `test/bundler/esbuild/`, the 5 of `test/bundler/css/`, `bundler_{allow_unresolved,banner,barrel,browser,bun,cjs,cjs2esm,comments,compile,compile_autoload,compile_prelinked,compile_splitting,decorator_metadata,defer,drop,dynamic_import_dce,edgecase,env,feature_flag,files,footer,html,html_server,jsx,loader,merged_enum,minify,minify_symbol_for,naming,plugin,plugin_chain,promiseall_deadcode,regressions,string}`, `html-import-manifest`, `metafile`, `cli`, `standalone`, `transpiler/react-compiler`, `transpiler/react-compiler-fixtures`, `regression/issue/25628`, `regression/issue/5344`, `bun-serve-html-manifest`, `bake/dev-and-prod`. - `bun-build-api`: 65 pass. 2 `bytecode:` tests time out at 5 s in a debug build here, the same on a debug build of main. - `bundler_bytecode_portable` times out in a debug build here. With `--bytecode`, the seven library bundles and `libraries.js` of its snapshot are byte for byte the same as on main. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 2 · 7 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 6 failed, 4 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/bundler_splitting.test.ts bun test v1.4.3 (c6b7fcb) test/bundler/bundler_splitting.test.ts: (pass) bundler > splitting/DynamicImportCSSFile [1119.69ms] (pass) bundler > splitting/DynamicImportMultipleCSSImports [758.58ms] (pass) bundler > splitting/StaticAndDynamicCSSImports [480.07ms] (pass) bundler > splitting/NestedDynamicImportWithCSS [621.16ms] (pass) bundler > splitting/SharedCSSBetweenChunks [613.19ms] (pass) bundler > splitting/DynamicImportChainWithCSS [597.55ms] (pass) bundler > splitting/ConditionalDynamicImportWithCSS [580.61ms] (pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [880.35ms] (pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [554.17ms] (pass) bundler > splitting/CircularDynamicImportsWithCSS [532.18ms] (pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [463.81ms] (pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [356.78ms] (pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [551.44ms] (pass) bundler > splitting/DeadDynamicImportTar ... (truncated) release without fix: 4 skipped bun test v1.4.3-canary.1 (5435db8) test/bundler/bundler_splitting.test.ts: (pass) bundler > splitting/DynamicImportCSSFile [20.71ms] (pass) bundler > splitting/DynamicImportMultipleCSSImports [12.29ms] (pass) bundler > splitting/StaticAndDynamicCSSImports [10.52ms] (pass) bundler > splitting/NestedDynamicImportWithCSS [11.94ms] (pass) bundler > splitting/SharedCSSBetweenChunks [14.78ms] (pass) bundler > splitting/DynamicImportChainWithCSS [12.19ms] (pass) bundler > splitting/ConditionalDynamicImportWithCSS [11.19ms] (pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [18.78ms] (pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [11.22ms] (pass) bundler > splitting/CircularDynamicImportsWithCSS [11.93ms] (pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [11.61ms] (pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [13.19ms] (pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [10.49ms] (pass) bundler > splitting/DeadDynamicImportTargetKeptWithoutTreeShaking [10.29ms] (pass) bundler > splitting/LiveDynamicImportTargetStillGetsChunk [12.87ms] (pass) bundler > splitting/DynamicImportChainLiveness [12.89ms] ( ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 4 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/bundler_splitting.test.ts bun test v1.4.3 (c6b7fcb) test/bundler/bundler_splitting.test.ts: (pass) bundler > splitting/DynamicImportCSSFile [816.60ms] (pass) bundler > splitting/DynamicImportMultipleCSSImports [486.26ms] (pass) bundler > splitting/StaticAndDynamicCSSImports [644.78ms] (pass) bundler > splitting/NestedDynamicImportWithCSS [551.52ms] (pass) bundler > splitting/SharedCSSBetweenChunks [585.99ms] (pass) bundler > splitting/DynamicImportChainWithCSS [544.72ms] (pass) bundler > splitting/ConditionalDynamicImportWithCSS [629.62ms] (pass) bundler > splitting/MultipleEntryPointsWithSharedCSS [748.73ms] (pass) bundler > splitting/DynamicImportWithOnlyCSSNoJS [474.70ms] (pass) bundler > splitting/CircularDynamicImportsWithCSS [597.88ms] (pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-cli [543.99ms] (pass) bundler > splitting/DeadDynamicImportTargetGetsNoChunk-api [357.83ms] (pass) bundler > splitting/DeadDynamicImportTargetNoSplittingReference [455.25ms] (pass) bundler > splitting/DeadDynamicImportTarg ... (truncated) release with fix: 4 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 629ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/14] cargo bun_runtime → libbun_runtime.a �[1m�[33mwarning�[0m�[1m: binary `bun_shim_impl` should have a kebab-case name�[0m �[1m�[94m|�[0m �[1m�[94m 1�[0m �[1m�[94m|�[0m /workspace/bun/build/release/rust-target/.../bun_shim_impl �[1m�[94m|�[0m �[1m�[33m^^^^^^^^^^^^^�[0m �[1m�[94m|�[0m �[1m�[94m= �[0m�[1mnote�[0m: `cargo::non_kebab_case_bins` is set to `warn` by default �[1m�[96mhelp�[0m: to change the binary name to `bun-shim-impl`, convert `bin.name` �[1m�[94m--> �[0msrc/install/windows-shim/Cargo.toml:41:8 �[1m�[94m|�[0m �[1m�[94m41�[0m �[91m- �[0mname = �[91m"bun_shim_impl"�[0m �[1m�[94m41�[0m �[92m+ �[0mname = �[92m"bun-shim-impl"�[0m �[1m�[94m|�[0m �[1m�[33mwarning�[0m: `bun_shim_impl` (manifest) generated 1 warning �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/bundler/Chunk.rs | 28 - src/bundler/LinkerContext.rs | 31 +- src/bundler/LinkerGraph.rs | 8 - src/bundler/linker_context/README.md | 9 +- .../findAllImportedPartsInJSOrder.rs | 1040 ++++++++++++++------ src/bundler/linker_context/mergeSmallChunks.rs | 2 +- test/bundler/bundler_splitting.test.ts | 730 ++++++++++++++ 7 files changed, 1489 insertions(+), 359 deletions(-) ``` </details> **gate history** · 7 passed · 1 rejected · iteration 2 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/bundler/Chunk.rs 1 1 72 src/bundler/LinkerContext.rs 6 4 72 src/bundler/LinkerGraph.rs 2 1 72 src/bundler/linker_context/README.md 2 2 72 …bundler/linker_context/findAllImportedPartsInJSOrder.rs 18 5 73 src/bundler/linker_context/mergeSmallChunks.rs 2 0 72 test/bundler/bundler_splitting.test.ts 10 10 72 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: Cliff <zylintw@gmail.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
--splitting,bun build --outdir out ./a.ts ./b.tssilently writes onlyout/b.jswhen the files import each other. CSS entry points lose a file the same way. A--compileexecutable can run nothing.compute_chunkskeys an entry chunk by the fullentry_bitsof its file (src/bundler/linker_context/computeChunks.rs:120). Entry points that reach each other get equal keys, so the second overwrites the first.aimports entryb,out/a.jscan runb's imports too early.Fix
LinkerGraph::loadkeeps one entry point per file.find_imported_parts_in_js_orderstarts its walk at the chunk's own entry point. HTML scripts now run in page order.module.exports, as in esbuild. This also fixes a re-export leak in single-entry builds. A missing ESM wrapper is no longer called (__INVALID__REF__();).test/bundler/. 9 fail on the base. Self-review: 3 concerns, all addressed.Background
entry_bitsis the set of entry points that reach a file.distance_from_entry_point, which is 0 for every entry point.Notes
Repro
bun build --outdir out ./a.ts ./b.tsprints onlyb.js (entry point). With--splitting, both files are written. Two CSS files that@importeach other lose one output, with and without--splitting.For
--compile:main.tsdoesimport("./plugin.ts")andplugin.tsdoesimport("./main.ts").bun build --compile ./main.ts ./plugin.ts --outfile appmakes an executable whose main module is the chunk ofplugin.ts. The code ofmain.tssits in aninit_mainwrapper that nothing calls, so./appprints nothing and exits 0.Module order
a.tsimportsc.ts, thenb.ts.b.tsimportsd.ts. Botha.tsandb.tsare entry points. Unbundled,bun a.tsprintsc d b a. On 1.4.1,out/a.jsprintsd b c a:b.tsalso has distance 0 and a lower stable index, so the walk starts there. esbuild printsc d b a. An HTML page with<script src="./first.js">and<script src="./second.js">runssecondfirst whensecond.jsis also an entry point.With only the key change, the cycle's
out/a.jswould get the module order ofout/b.js. So the key change and the walk change stay together.module.exportsin cjs outputTwo writes to
module.exportscome from the linker, not from user code:module.exports = __toCommonJS(exports)at the end of part 0 of each entry point file (doStep5.rs).export * froma module that needs a run-time re-export prints__reExport(exports, mod, module.exports)(convertStmtsForChunk.rs).Both are per file, and the file of one entry point can print in the chunk of another. The walk emits part 0 of a file when it enters the file, before its imports. On 1.4.1 the other entry point's file came first, so its writes came before the chunk's own assignment and did no harm. With the walk change, they come last. For
index.tsthat importsutils.ts, both entry points,require("./out/index.js")returned the exports ofutils.ts. In a cycle, 1.4.1 already got this wrong: its one output,b.js, exported the names ofa.ts.Now both writes happen only when the file is the entry point of the chunk (
Chunk::is_entry_point_file). esbuild has the same rule (file.IsEntryPoint(), in a link of one entry point). The part 0 write is the last statement of that part, and the printer drops it in other chunks.The re-export rule also changes single-entry builds. On 1.4.1, a file that the entry point imports, but does not re-export, still wrote its
export * from "ext"names ontomodule.exports. esbuild does not. A re-export chain still works, because each file in it re-exports the next file at run time, and the entry point file writes tomodule.exports(checked with a chain of three files).importstar/ReExportStarEntryPointAndInnerFileExternalexpected the leak. Before 2f7ff95, the harness threw for every cjs test, and this test expected the value that esbuild prints. That commit set the current value when cjs output first ran. The same commit addedReExportStarEntryPointAndInnerFile, without externals, which expects no top-levelb. This PR restores the esbuild value.Two entry points that name one file
An onResolve plugin can map two entry point names to one file. The plugin path adds the file to the entry points twice. On 1.4.1, the full-bits key merged the two chunks without
--splitting, so the build wrotesecond-name.js. With--splitting, it failed with "Multiple files share the same output path".LinkerGraph::loadnow skips a file that is already an entry point, as its loop forimport()targets does. Both modes writesecond-name.js. The name comes fromentry_point_original_names, which keeps the last name.When a plugin declines an entry point and the default resolver finds a file that is already an entry point,
enqueue_entry_itemreturnsNoneand records no name. This PR does not change that path. #38600 works on it.Output order
The old non-split key started with a byte that was 0 for an HTML entry point and 1 for the others. It sorted HTML entry chunks before JS entry chunks. The own-bit key has no such byte. For
bun build ./index.html ./b.ts ./c.html, the outputs are now listed asindex.html,b.js,c.html(with their chunks), in the order of the entry points. The files and their contents do not change. Entry point 0 still sorts first, so--compilepicks the same main module.Tests
New, and failing on the base commit:
edgecase/EntryPointsImportEachOther,edgecase/EntryPointImportsEntryPointModuleOrder,edgecase/CSSEntryPointsImportEachOther,edgecase/EntryPointsImportEachOtherCommonJShtml/script-that-is-also-an-entry-pointcompile/EntryPointsImportEachOtherplugin/two entry points that resolve to one file (splitting: true)edgecase/EntryPointRequiredByEntryPointCommonJS,edgecase/RequiredEntryPointWithoutWrapperCommonJSNew, and passing on the base commit:
edgecase/EntryPointImportsEntryPointCommonJSand thesplitting: falseplugin case. Both fail if either cjs write or the dedupe is removed.Changed:
importstar/ReExportStarEntryPointAndInnerFileExternal(above) andcompile/HelloWorldWithProcessVersionsBun. A debug build inlinesprocess.versions.bunas1.4.1-debug. That test removed-debugonly from the expected value, so it failed on every debug build. The API variant of the test removes it from both values, and now this one does too.Checked by hand
Against esbuild 0.25.1 (run output and
module.exportscompared): a ring of three entry points inesm,cjs, andiife, a CommonJS cycle,export * fromanother entry point, re-export chains of two and three files in cjs, and a named import from a file that re-exports anode:module. Also rings of 10 and 130 entry points (130 uses the heap bit set),Bun.buildwithmetafileandsourcemap: "external", an HTML entry point whose script is in a cycle of two JS entry points, and--no-bundle --format=cjs(byte-identical output).Suites run with the debug build
bundler_edgecase,bundler_html,bundler_html_server,bundler_plugin,bundler_cjs,bundler_compile,bundler_compile_splitting,bundler_splitting,esbuild/{default,importstar,importstar_ts,dce,splitting,css,metafile,loader,lower,ts,extra,packagejson},metafile,bundler_naming,html-import-manifest,bundler_cjs2esm,bundler_regressions,bun-build-api,bundler_browser,bundler_bun,cli,bundler_loader,bundler_files,bundler_barrel,bundler_minify,regression/issue/5344, andbake/{dev/production,dev-and-prod,dev/ssg-pages-router}. An earlier round also ranbun-build-compile,compile-asset-bunfs,native-plugin, andregression/issue/{10004,10139,22317,25716,27431,32686}.Some tests time out at 5 s on the debug build. Each of them builds one entry point, and each passes with a longer timeout.
__INVALID__REF__();in cjs outputA file that something
require()s gets an ESM wrapper (init_foo). When all of its top-level statements hoist, the parser gives it no wrapper symbol (needs_wrapper_ref), and the printer prints the file with no wrapper. The esm tail of an entry point checks for the symbol. The cjs tail did not, so it printed__INVALID__REF__();, and loading the output threwReferenceError: __INVALID__REF__ is not defined. The cjs tail now has the same check.One entry point is enough:
entry.tshasexport function load() { return require("./c.ts"); }, andc.tsrequiresentry.ts. With two entry points,a.tsdoesrequire("./b.ts"), andb.tshas onlyexport constlines. 1.2.0, 1.3.0, 1.4.0, and 1.4.1 all print the bad call.#37843 has the same check, and it also adds the missing iife tail.
bun build --format=iifeof a CommonJS entry point runs none of its code (1.2.0 through main), because nothing callsrequire_entry(). This PR does not change iife output.Related
--splitting. It does not touch the non-split key.import.meta.mainprints when a build has several entry points.