bundler: error instead of panic when splitting is used with non-ESM format - #32685
Conversation
…ormat Code splitting with format "cjs" or "iife" would panic in compute_cross_chunk_dependencies: exports_to_other_chunks is only populated for ESM, so sorted_cross_chunk_imports hit None.unwrap() when a shared chunk was produced. Reject the combination in BundleV2::init so both the CLI and Bun.build paths surface a build error instead. Matches esbuild. Fixes #32395
|
Updated 2:17 AM PT - Jun 25th, 2026
❌ @robobun, your commit 044b135 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 32685That installs a local version of the PR into your bun-32685 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
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 (1)
💤 Files with no reviewable changes (1)
WalkthroughBundle initialization now logs an error when code splitting is enabled without an ES module output format. A regression test covers failed bundling for cjs and iife across cli and api backends and checks the expected error message. ChangesCode splitting format validation
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
LGTM — straightforward validation that turns a panic into a clean build error, matching esbuild.
Extended reasoning...
Overview
This PR adds a single validation check in BundleV2::init (src/bundler/bundle_v2.rs) that emits a build error when code_splitting is enabled with a non-ESM output_format. Previously this combination would proceed into the linker and panic on a None.unwrap() in sorted_cross_chunk_imports because exports_to_other_chunks is only populated for ESM. The error is logged via transpiler.log_mut().add_error(...), and I verified that all callers of init (both the CLI/normal build path at line ~3791/3929 and the bake-production path at line ~3970) immediately check log().has_errors() and convert it to a BuildFailed error, so the build aborts cleanly without reaching the panic site. Four new test cases in test/bundler/esbuild/splitting.test.ts cover cjs/iife × cli/api and assert the exact error message via bundleErrors.
Security risks
None. This is purely an input-validation guard that rejects an option combination earlier than before. No new I/O, parsing, or privilege-bearing code paths are introduced.
Level of scrutiny
Low. The change is 13 lines of additive Rust (a conditional + log call) and ~30 lines of test code that follow existing patterns in the same file. The combination being rejected never produced working output (it panicked), so there is no risk of regressing existing valid use cases. I checked that code_splitting = true is only set via the user-facing --splitting flag / splitting JS option, so the check cannot accidentally trip for the internal InternalBakeDev format (which doesn't enable code splitting). The error message and behavior mirror esbuild's, which is the reference implementation for this bundler.
Other factors
No CODEOWNERS cover the touched files. No outstanding reviewer comments. The PR description includes a clear root-cause analysis and reports passing test runs for the splitting and CJS bundler suites. The bug-hunting system found no issues.
There was a problem hiding this comment.
LGTM — small, well-tested validation that turns a panic into a clear build error, matching esbuild.
Extended reasoning...
Overview
This PR adds a 13-line guard in BundleV2::init (src/bundler/bundle_v2.rs) that emits a build error when code_splitting is enabled with any output format other than Esm, plus a 30-line regression test in test/bundler/esbuild/splitting.test.ts covering cjs/iife × cli/api (4 cases). Previously this combination panicked on None.unwrap() in sorted_cross_chunk_imports (or produced broken output per #15125); now it fails cleanly with the same message esbuild uses.
Security risks
None. This is a user-facing config validation in the bundler — no auth, crypto, permissions, injection surface, or data handling involved. The change strictly narrows behavior from panic/garbage-output to a catchable error.
Level of scrutiny
Low. The change is small, mechanical, and self-contained:
add_error(None, Loc::EMPTY, msg)is an established pattern (e.g.generateChunksInParallel.rs:439).- Both entry paths check
log().has_errors()afterinit(verified atbundle_v2.rs:3791and:3929), and the new tests confirm the error surfaces on both CLI and JS API backends. - Internal callers that set
code_splitting = true(bake production, HTMLBundle serve) already useFormat::Esm— bake only usesInternalBakeDevin development mode wherecode_splitting = false— so no internal regression. - The combination never produced working output, so no functionality is being removed.
Other factors
No CODEOWNERS for src/bundler/. The PR description includes verified test runs (26 pass / 2 pre-existing todo in splitting.test.ts, plus the related bundler_splitting and bundler_cjs suites). The one inline nit (extra bug-history comment line in the test per CLAUDE.md style) is cosmetic and doesn't block approval.
There was a problem hiding this comment.
LGTM — thanks for dropping the extra comment line.
Extended reasoning...
Overview
This PR adds a single guard in BundleV2::init (src/bundler/bundle_v2.rs) that emits a build error when code_splitting is enabled with any output_format other than Esm, replacing a downstream Option::unwrap() panic in sorted_cross_chunk_imports. It also adds four regression tests in test/bundler/esbuild/splitting.test.ts (cjs/iife × cli/api) asserting the new error message via the existing bundleErrors harness.
Security risks
None. The change is pure input validation on bundler config flags — it adds an early log_mut().add_error(...) call with a static message string. No parsing, no I/O, no auth/crypto/permission surface. The only behavioral change is that an unsupported flag combination now fails cleanly instead of panicking.
Level of scrutiny
Low. The diff is ~13 lines of Rust and ~29 lines of test fixture. I verified:
add_errorflows into the existinghas_errors()→BuildFailedchecks at bundle_v2.rs:3791/3929/3970, so both CLI and JS API paths surface it as a build failure as described.options::Format::Esmmatches the enum usage already present a few lines above (line 2784,InternalBakeDev).code_splitting = trueis only set from the user-facing--splittingflag /splittingJS option — no internal path (e.g. Bake dev server withInternalBakeDev) enables it, so the new guard can't regress internal callers.- The error wording mirrors esbuild's equivalent rejection.
- Neither changed file is covered by CODEOWNERS.
Other factors
My only prior feedback (drop the bug-history comment line per CLAUDE.md) was addressed in 044b135 and the thread is resolved. The bug-hunting system found no issues on this revision. The PR description includes passing test runs for the touched suite plus adjacent splitting/cjs suites. This is a textbook panic→clean-error conversion with regression coverage on both entry points.
|
CI on build 64590 finished with 281 jobs passed and 5 failed, none related to this diff:
The flaky-retry annotations are |
#39445) Stacked on #36463 (the base branch is that PR's branch, so the diff here is only the additions). Merging this into #36463 adds the behavior changes listed below; #36463 itself now covers the #38333 install batch, the optional-peer correction, and the TOML / `bun init` fixes, so this PR no longer touches those. ### Problem - These 1.3 to 1.4 behavior changes are not in the guide at `701b3e2a0`: - MySQL: the first `caching_sha2_password` connection over plain TCP is refused unless `allowPublicKeyRetrieval: true` (#31129; 1.3.14 requested the key automatically, `MySQLConnection.zig` in the 1.3.14 tag). SQL `tls` / `ssl` options now require TLS instead of falling back to plaintext, and `?ssl=` / `?ssl-mode=` are read (`shared.ts` 1.3.14 only read `?sslmode=`; #37669). - Install: `~/.npmrc` fallback when `XDG_CONFIG_HOME` is set (#36289), credentials in `--registry` / env / bunfig object URLs are sent and outrank same-host `.npmrc` tokens (#38796, #38824), `bun outdated` exits 1 on fetch failures (#38809), new `dedupe` / `up` commands shadow scripts of those names and `bun feedback` is removed (#38333, #38444), `workspace:` ranges inside registry packages (#37669), isolated store entry names (#39014). - Runtime: `module.enableCompileCache()` / `NODE_COMPILE_CACHE` implemented (#34660), `require()` / `import` not-found messages (#34660), `AbortError` message without the period (#39277; 1.3.14's `BunCommonStrings.h` has the period), GCM IV length (#34092), `mkdtemp("")` (#34908), vm options (#38381), `server.reload` (#38697), ICU 75/73 to 78 (#38013), Compression stream chunking (#38695), `Bun.SQL` sqlite bindings (#35950). - Bundler: `splitting` with `cjs` / `iife` is an error (#32685), block-scoped `enum` lowers to `let` (#34249), exports emitted ascending instead of descending (#35957; `doStep5.zig` in 1.3.14 used `sortDesc`), minified `$` (#35668). - The TOML integer bullet did not say what the limit or the fix is. ### Fix - Adds a MySQL public key section (plus a summary table row), a TLS note under the `PGSSLMODE` section, an `.npmrc` / credentials addendum to the `bunfig.toml` section, a `module.enableCompileCache()` section, and the rest as bullets in the existing lists. - `docs/pm/overrides.mdx`: one-line change adding a pointer to this guide in the existing `lockfileVersion` 3 limitation. (The base branch briefly had a duplicate "Nested overrides" section; it removed that itself in `8257d01acb`, and this PR was rebased over it.) - Verification: each runtime claim was run against `1.4.0-canary.1+8326d1bd3` (22 commits behind main; contains every change referenced), and each install or bundler claim was checked against the source on main, with the 1.3 side taken from the `bun-v1.3.14` tag where the PR body did not state it. The `/runtime/sql#mysql` and `/upgrade-to-1.4` links resolve. `prettier --check` passes. ### Not included on purpose - Lifecycle scripts no longer receiving `npm_package_name` / `npm_package_version` / `npm_package_json` / `npm_config_local_prefix` during `bun install`, and transitive `"*"` ranges no longer deduplicating onto the root's version: regressions with open fixes (#36690, #38110, #38770). They need either the fixes or a guide line before release. - Postgres `sslmode=prefer` / `allow` (including `PGSSLMODE=prefer`, which 1.4 newly reads) hangs until the connection timeout against a server without SSL because nothing sends the startup message after the `N` reply. Same code in 1.3.14; filed as a bug instead of documented. <details> <summary>Commands used to verify the runtime claims</summary> ``` timers/promises setTimeout with an aborted signal # "The operation was aborted" bun req.cjs # Cannot find module ... Require stack: bun b.mjs (import() of a missing package / relative file) # Cannot find package 'x' imported from /path, ERR_MODULE_NOT_FOUND bun a_static.mjs (unhandled static import) # printed line still: Cannot find package 'x' from '/path' process.versions.icu # 78.3 createCipheriv("aes-128-gcm", key, Buffer.alloc(129)) # ERR_CRYPTO_INVALID_IV DecompressionStream of a 1 MiB gzip member # 16 chunks of 65536 bytes new SQL("sqlite://:memory:") with ${[1,2]} / ${new Date()} # Binding expected ... fs.mkdtempSync("") # EINVAL vm.runInThisContext("1", []) # ERR_INVALID_ARG_TYPE NODE_COMPILE_CACHE=/tmp/cc bun cc.cjs # creates /tmp/cc/v1.4.0-x86_64-<sha>-<uid> NODE_DISABLE_COMPILE_CACHE=1 + enableCompileCache() # status 3 (DISABLED) bun dedupe / bun up with package.json scripts of those names # built-in command runs bun feedback # Script not found "feedback" Bun.build({ splitting: true, format: "cjs" }) # Code splitting is currently only supported ... bun build of a function-scoped enum and import * as ns # let Color; exports a, m, z new SQL({ url: "postgres://...", tls: true }) on a non-TLS server # ERR_POSTGRES_TLS_NOT_AVAILABLE Bun.TOML.parse("a = 9007199254740993") # Integer cannot be losslessly represented ... ``` </details> <details> <summary>Previous revision</summary> The first revision of this PR (`3c5611454a`) also rewrote the package manager section for #38333 / #38853 (nested overrides and `lockfileVersion: 3`, the optional-peer correction, `bun update`, `bunfig.toml` over `.npmrc`, `--filter`) and fixed the TOML date and `bun init` lines. #36463 picked those up in its own commits the same day, so this PR was rebased onto its new head and reduced to the items above. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · docs-only change; test-proof not applicable <!-- robobun:evidence:end -->
#39445) Stacked on #36463 (the base branch is that PR's branch, so the diff here is only the additions). Merging this into #36463 adds the behavior changes listed below; #36463 itself now covers the #38333 install batch, the optional-peer correction, and the TOML / `bun init` fixes, so this PR no longer touches those. ### Problem - These 1.3 to 1.4 behavior changes are not in the guide at `701b3e2a0`: - MySQL: the first `caching_sha2_password` connection over plain TCP is refused unless `allowPublicKeyRetrieval: true` (#31129; 1.3.14 requested the key automatically, `MySQLConnection.zig` in the 1.3.14 tag). SQL `tls` / `ssl` options now require TLS instead of falling back to plaintext, and `?ssl=` / `?ssl-mode=` are read (`shared.ts` 1.3.14 only read `?sslmode=`; #37669). - Install: `~/.npmrc` fallback when `XDG_CONFIG_HOME` is set (#36289), credentials in `--registry` / env / bunfig object URLs are sent and outrank same-host `.npmrc` tokens (#38796, #38824), `bun outdated` exits 1 on fetch failures (#38809), new `dedupe` / `up` commands shadow scripts of those names and `bun feedback` is removed (#38333, #38444), `workspace:` ranges inside registry packages (#37669), isolated store entry names (#39014). - Runtime: `module.enableCompileCache()` / `NODE_COMPILE_CACHE` implemented (#34660), `require()` / `import` not-found messages (#34660), `AbortError` message without the period (#39277; 1.3.14's `BunCommonStrings.h` has the period), GCM IV length (#34092), `mkdtemp("")` (#34908), vm options (#38381), `server.reload` (#38697), ICU 75/73 to 78 (#38013), Compression stream chunking (#38695), `Bun.SQL` sqlite bindings (#35950). - Bundler: `splitting` with `cjs` / `iife` is an error (#32685), block-scoped `enum` lowers to `let` (#34249), exports emitted ascending instead of descending (#35957; `doStep5.zig` in 1.3.14 used `sortDesc`), minified `$` (#35668). - The TOML integer bullet did not say what the limit or the fix is. ### Fix - Adds a MySQL public key section (plus a summary table row), a TLS note under the `PGSSLMODE` section, an `.npmrc` / credentials addendum to the `bunfig.toml` section, a `module.enableCompileCache()` section, and the rest as bullets in the existing lists. - `docs/pm/overrides.mdx`: one-line change adding a pointer to this guide in the existing `lockfileVersion` 3 limitation. (The base branch briefly had a duplicate "Nested overrides" section; it removed that itself in `8257d01acb`, and this PR was rebased over it.) - Verification: each runtime claim was run against `1.4.0-canary.1+8326d1bd3` (22 commits behind main; contains every change referenced), and each install or bundler claim was checked against the source on main, with the 1.3 side taken from the `bun-v1.3.14` tag where the PR body did not state it. The `/runtime/sql#mysql` and `/upgrade-to-1.4` links resolve. `prettier --check` passes. ### Not included on purpose - Lifecycle scripts no longer receiving `npm_package_name` / `npm_package_version` / `npm_package_json` / `npm_config_local_prefix` during `bun install`, and transitive `"*"` ranges no longer deduplicating onto the root's version: regressions with open fixes (#36690, #38110, #38770). They need either the fixes or a guide line before release. - Postgres `sslmode=prefer` / `allow` (including `PGSSLMODE=prefer`, which 1.4 newly reads) hangs until the connection timeout against a server without SSL because nothing sends the startup message after the `N` reply. Same code in 1.3.14; filed as a bug instead of documented. <details> <summary>Commands used to verify the runtime claims</summary> ``` timers/promises setTimeout with an aborted signal # "The operation was aborted" bun req.cjs # Cannot find module ... Require stack: bun b.mjs (import() of a missing package / relative file) # Cannot find package 'x' imported from /path, ERR_MODULE_NOT_FOUND bun a_static.mjs (unhandled static import) # printed line still: Cannot find package 'x' from '/path' process.versions.icu # 78.3 createCipheriv("aes-128-gcm", key, Buffer.alloc(129)) # ERR_CRYPTO_INVALID_IV DecompressionStream of a 1 MiB gzip member # 16 chunks of 65536 bytes new SQL("sqlite://:memory:") with ${[1,2]} / ${new Date()} # Binding expected ... fs.mkdtempSync("") # EINVAL vm.runInThisContext("1", []) # ERR_INVALID_ARG_TYPE NODE_COMPILE_CACHE=/tmp/cc bun cc.cjs # creates /tmp/cc/v1.4.0-x86_64-<sha>-<uid> NODE_DISABLE_COMPILE_CACHE=1 + enableCompileCache() # status 3 (DISABLED) bun dedupe / bun up with package.json scripts of those names # built-in command runs bun feedback # Script not found "feedback" Bun.build({ splitting: true, format: "cjs" }) # Code splitting is currently only supported ... bun build of a function-scoped enum and import * as ns # let Color; exports a, m, z new SQL({ url: "postgres://...", tls: true }) on a non-TLS server # ERR_POSTGRES_TLS_NOT_AVAILABLE Bun.TOML.parse("a = 9007199254740993") # Integer cannot be losslessly represented ... ``` </details> <details> <summary>Previous revision</summary> The first revision of this PR (`3c5611454a`) also rewrote the package manager section for #38333 / #38853 (nested overrides and `lockfileVersion: 3`, the optional-peer correction, `bun update`, `bunfig.toml` over `.npmrc`, `--filter`) and fixed the TOML date and `bun init` lines. #36463 picked those up in its own commits the same day, so this PR was rebased onto its new head and reduced to the items above. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · docs-only change; test-proof not applicable <!-- robobun:evidence:end -->
What does this PR do?
Fixes #32395.
Fixes #15125.
Reproduction
Same via
Bun.build({ entrypoints: ["./a.ts", "./b.ts"], outdir: "./dist", format: "cjs", splitting: true }).--format iifepanics identically.--format esmworks.Root cause
CrossChunkImport::sorted_cross_chunk_imports(src/bundler/Chunk.rs:1548) looks up each cross-chunk import in the target chunk'sexports_to_other_chunksmap:exports_to_other_chunksis only populated whenoutput_format == Esm(computeCrossChunkDependencies.rs, thematch c.options.output_format { Esm => { ... } _ => {} }arm), whileimports_from_other_chunksis populated unconditionally earlier in the same function. So when CJS/IIFE + splitting produces a shared chunk, each entry chunk has import items whose refs are looked up in an always-empty map, andNone.unwrap()panics.Fix
The linker has never implemented cross-chunk imports/exports for non-ESM formats (both
match output_formatarms are_ => {}for CJS/IIFE), so the combination has never produced working output. esbuild rejects it outright:This PR adds the same validation in
BundleV2::init, which bothbun build(CLI) andBun.build(JS API) go through, and which both follow with alog().has_errors()check that converts it into a build failure. The panic becomes a catchable build error:Verification
New tests in
test/bundler/esbuild/splitting.test.tscovercjsandiifeon both thecliandapibackends (4 cases). On the unfixed build the CLI cases fail with'bun build' subprocess killed by SIGILLand the API case panics the test process; with the fix all four pass with the expected error message.bun bd test test/bundler/esbuild/splitting.test.ts: 26 pass, 2 todo (pre-existing)bun bd test test/bundler/bundler_splitting.test.ts: 10 passbun bd test test/bundler/bundler_cjs.test.ts: 23 pass