bundler: read .default of a split require() whose target is CommonJS at link time

[robobun]

Problem

• With --splitting --target=bun, a split require() of a file that is CommonJS at link time returns the chunk namespace { default: module.exports } and not module.exports. In the repro of bundler: split require() of a re-wrapped lifted module returns the chunk namespace (regression on main) #41236, require("react-dom").version is undefined. With npm react-dom 18.3.1 in development, createPortal is undefined.
• The cause: since bundler: lift module.exports = require() after side effects in unwrapped packages to export * #41188 the parser lifts module.exports = require("./impl") into export * from "./impl", so the file is ESM at parse time and ReachableFileVisitor splits the require() (src/bundler/bundle_v2.rs:1998). The linker then wraps the file again because ./impl is CommonJS (scanImportsAndExports.rs:422). Its chunk is export default require_x(), and import.meta.require() of that chunk returns the namespace. This is a regression on main only. No release has it.

Fix

• scan_imports_and_exports sets a new record flag, CROSS_CHUNK_REQUIRE_DEFAULT, on a split require() whose target has exports_kind == Cjs at link time. The printer then prints import.meta.require("./chunk.js").default.
• Correct because splitting is ESM output only, where a CommonJS entry chunk exports exactly one thing: default: module.exports. A target that stays ESM at link time keeps the bare call and gets its namespace, as before.
• The split output now matches the unsplit bundle: typeof m, m.version, typeof m.default print object 19.0.0 function in both.
• Verified: two new cases in test/bundler/bundler_splitting.test.ts (SplitRequireOfRewrappedLiftedCommonJS#41236 fails on main). Also bundler_splitting, bundler_cjs2esm, bundler_cjs, esbuild/splitting, bundler_compile_splitting, bundler_edgecase, bundler_regressions, bundler_bun, all green with the debug build.

Background

• A split require() (target bun, --splitting) makes its target an entry point with its own chunk and prints the call as import.meta.require(path). The linker decides this at parse time from ExportsKind::Esm.
• Lifting: in an ESM bundle the parser turns top-level exports.foo = ... or module.exports = require() into ES exports. The lifted file can still become CommonJS at link time, for example when the target of its export star has no static exports.
• bundler: wrap a split import() with __toESM when the target is CommonJS at link time #41237 fixes the same shape for a split import(). This PR changes only require() records.

Notes

Repro from the issue, in an empty directory:

mkdir -p node_modules/react-dom
printf "console.log('side effect');\nmodule.exports = require('./impl');\n" > node_modules/react-dom/index.js
printf 'module.exports = function render() { return "rendered"; };\nmodule.exports.version = "19.0.0";\n' > node_modules/react-dom/impl.js
printf 'let m;\ntry {\n  m = require("react-dom");\n} catch {}\nconsole.log(m.version);\n' > entry.js
bun build ./entry.js --splitting --target=bun --outdir=out && bun out/entry.js

Main prints side effect then undefined. With this change it prints 19.0.0. The entry chunk now has m = import.meta.require("./index-<hash>.js").default.

module.exports of the re-wrapped lifted file is __toESM(require_impl()), an object with default set to the function. That is the #41188 lift behavior and is the same in the unsplit bundle, so the test reads m.default().

The second test, SplitRequireOfLiftedCommonJSStaysEsm, pins the other side: when ./impl uses exports.x = ... and is lifted too, the chunk stays ESM and the call has no .default.

The alternative, to not split a require() of a lifted file with an export star, would keep the wrapper in the importer's chunk. The link-time check is the smaller change and covers every way a parse-time ESM target becomes CommonJS at link time, not only the lift case.

cargo clippy -p bun_bundler -p bun_js_printer is clean.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 18 days. After that, they cost $0.25 per reviewed file.}

Or wait 12 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 73828113-8603-4308-8d97-8f852d031980

📥 Commits

Reviewing files that changed from the base of the PR and between dcd8569 and 751b74e.

📒 Files selected for processing (4)

• src/ast/import_record.rs
• src/bundler/linker_context/scanImportsAndExports.rs
• src/js_printer/lib.rs
• test/bundler/bundler_splitting.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

I reviewed this PR and didn't find any bugs. The fix is small and mirrors the existing import() cross-chunk-CJS handling right below it, but since it changes bundler code generation (linker + printer) for split require() output, a human familiar with the splitting/re-wrap semantics should confirm.

What was reviewed:

• The new flag condition in scan_imports_and_exports — is_external_dyn already implies rec_source_index.is_valid(), and the exports_kind[...] indexing matches the sibling dynamic-import block at lines 1158-1168.
• Printer placement of .default — WRAP_WITH_TO_ESM is never set for ImportKind::Require in this branch, so there's no suffix-ordering interaction; the outer wrap paren still closes after .default.
• Bitflag value 1 << 18 is the next free bit after NAMESPACE_UNUSED.
• Both tests use the existing chunkContaining helper and assert runtime output plus emitted shape; the negative test pins that an ESM-at-link-time target gets no .default.

Extended reasoning...

Overview

This PR fixes a bundler regression introduced by #41188 (the module.exports = require() lift). When --splitting --target=bun splits a require() whose target was lifted to ESM at parse time but re-wrapped as CommonJS at link time, the emitted import.meta.require("./chunk.js") returned the chunk's namespace object { default: module.exports } instead of module.exports. The fix adds a new ImportRecordFlags::CROSS_CHUNK_REQUIRE_DEFAULT bit (1 << 18), sets it in scan_imports_and_exports when a split require() targets a source whose exports_kind is Cjs at link time, and has the printer append .default to the emitted call. Two itBundled tests cover the re-wrapped-CJS case (issue #41236 repro) and the stays-ESM negative case.

Security risks

None. This is bundler output shaping — no user-input parsing, no auth/crypto/permissions, no network. The change only affects what property access is emitted after a synthetic import.meta.require() call in split ESM output.

Level of scrutiny

Moderate. The diff is ~30 lines of production code and is structurally identical to the neighboring dynamic-import handling (lines 1158-1168 in scanImportsAndExports.rs), which already checks the same is_external_dyn && exports_kind == Cjs condition for import(). The redundant rec_source_index.is_valid() guard is harmless (already implied by is_external_dyn per line 1069). In the printer, .default is appended after the call and before the wrap_with_to_esm suffix and outer wrap paren; since WRAP_WITH_TO_ESM is only ever set for kind != Require in this code path, there's no ordering conflict. However, this is still a code-generation change in the bundler's linker/printer, where the correctness of "the chunk's only export is default: module.exports" depends on how CJS entry chunks are emitted under splitting — a reviewer who knows that emission path should confirm the invariant holds for all re-wrap shapes, not just the lifted-module.exports = require() one.

Other factors

Tests are placed correctly in test/bundler/bundler_splitting.test.ts using itBundled, follow existing patterns (chunkContaining helper, onAfterBundle shape assertions, run: for runtime output), and cover both the positive and negative branches of the new condition. The PR description states the broader bundler suites (splitting, cjs2esm, cjs, esbuild/splitting, compile_splitting, edgecase, regressions, bun) pass. No CODEOWNERS cover these files. The bug hunt exited on dry_streak with no findings. Given the change touches core bundler output rather than being a mechanical tweak, deferring for a human sign-off is the safer call.

[robobun]

On the invariant that a CommonJS entry chunk exports only default: module.exports: generate_entry_point_tail_js (src/bundler/linker_context/postProcessJSChunk.rs:844) emits exactly export default require_foo(); for every WrapKind::Cjs entry in ESM output, with no other shape. Step 1 of scan_imports_and_exports sets wrap = Cjs for every file with exports_kind == Cjs when the output format is ESM, and splitting is ESM only. So the .default read holds for every way a target becomes CommonJS at link time, not only the lifted module.exports = require() case.