Repository navigation
Conversation
…ifted CommonJS file A `require()` that stays a call (inside `try`/`catch`, or through an alias of the package) of a CommonJS file that the parser lifted to an ES module evaluated to `__toCommonJS(exports_x)`: a read-only copy with an `__esModule` property that the file never set. With `--splitting --target=bun` the call was `import.meta.require(chunk)`, which returns the module namespace of the chunk with the same property. The parser now records when a lifted file is CommonJS to `bun run` (`NAMESPACE_IS_MODULE_EXPORTS`): a top-level `exports.x = ...` became an export, or a `require()` became an import and the file is not an ES module by type. For such a target the linker does not wrap the call in `__toCommonJS`, and `find_reachable_files` does not split the call. The call evaluates to `(init_x(), exports_x)`, the namespace object that an unwrapped `require()` and a default import already use.
…onJS file `require_returns_namespace` now tests `COMMONJS_LIFTED_TO_ESM` on a file that is still `ExportsKind::Esm`, and the parser flag `NAMESPACE_IS_MODULE_EXPORTS` is gone. The step 6 check applies to ESM output only, because no other format lifts. A cycle of files that re-export each other counts as lifted CommonJS. The helper calls `get_redirect_id` for a redirect file, and the debug assertion sits above the comment of the `.default` arm. Tests: each case also runs its entry point with `bun run` and compares the output. New cases cover the re-export file of react-dom, a cycle, IIFE output, an alias and a second entry point under code splitting, and a compiled executable. Two `todo` cases record what is still wrong: a write through a re-export file, and a lifted re-export of an external package under code splitting. Docs: the added sentences say only what runs on real React.
…l re-export control Another entry point that imports a file of the lifted target's import closure now calls its wrapper in import order. A lifted `module.exports = require()` of an external package stays a chunk boundary: with an `import()` of the same file the call loads that chunk.
…-require-lifted-commonjs-namespace
…mpile file as it is A file that only calls require() in a package with "type": "module" is an ES module to Bun. The bundle treats it as a lifted CommonJS file.
… on main The change now covers a `require()` that stays a call in the same chunk as its lifted target. Step 6 of `scan_imports_and_exports` adds no `__toCommonJS` copy there, and the call evaluates to the namespace object of the file. - `find_reachable_files` is back at main. With `--splitting --target=bun` a lifted file is a chunk again, and the call there keeps the value of main. - `require_returns_namespace` also accepts a lifted CommonJS file of a package outside the unwrap list as the target of a lifted `module.exports = require()`. - The cases are in `test/bundler/bundler_cjs2esm.test.ts`. `test/bundler/bundler_splitting.test.ts` is the file of main again. - `matchesBunRun` is an option of the bundler test harness. It replaces the two local helpers.
…odule.exports" With `--splitting --target=bun`, a `require()` of a lifted CommonJS file that stays a call loads the chunk of the file with `import.meta.require`. That call returned the module namespace of the chunk, which is read-only and has `__esModule: true`. The chunk of such a file now exports its namespace object under the name "module.exports". The loader of Bun returns that export from `require()` of an ES module, so the call evaluates to the object. The linker marks the file in step 1 of `scan_imports_and_exports`, when a split `require()` does more than read named exports off its result.
Collaborator
Author
|
Updated 12:15 PM PT - Oct 28th, 2026
❌ @robobun, your commit 5a021a3 has 6 failures in
🧪 To try this PR locally: bunx bun-pr 47193That installs a local version of the PR into your bun-47193 --bun |
Collaborator
Author
|
Status: draft. It waits for a maintainer to choose between the two remedies in the PR body (keep the chunk and export Repro. The same call as in #47080, with mkdir -p node_modules/react && echo '{}' > package.json
echo '{"name":"react","version":"19.0.0","main":"./index.js"}' > node_modules/react/package.json
printf 'exports.count = 0;\nexports.inc = () => ++exports.count;\n' > node_modules/react/index.js
cat > entry.js <<'EOS'
let m;
try { m = require("react"); } catch {}
let write = "ok";
try { m.count = 41; } catch (e) { write = e.constructor.name; }
console.log(m.__esModule, write, m.inc());
EOS
bun entry.js # undefined ok 42
bun build --splitting --target=bun entry.js --outdir=out && bun out/entry.js
# main: true TypeError 1CI. Buildkite build 134479 passed. Its six red jobs are the soft-fail PR: #47193 |
This branch has not been deployed
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
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.
Stacked on #47080. Draft until a maintainer answers the question under Background.
Problem
--splitting --target=bun,require("react-dom/server")in atrybody returns the module namespace of a chunk: a read-only object with__esModule: true. bundler: skip the__toCommonJScopy for require() of a lifted CommonJS file #47080 fixes the other modes..defaultasundefined. A write throwsTypeError: Attempted to assign to readonly property.import.meta.require("./chunk.js"), and the chunk of a lifted file exports only its names.Fix
"module.exports". Bun's loader returns that export fromrequire()of an ES module (src/js/builtins/CommonJS.ts, since 1.2.6).require()does more than read named exports. The printed call does not change.test/bundler/bundler_splitting.test.ts, 18 fail before. Self-reviewed: 16 concerns on this route, 9 addressed, 7 open (Notes).Background
require("react-dom/server")costs 44.0M startup instructions, not 4.0M.reactand six more packages, the parser turnsexports.x = vinto an ES export. A splitrequire()loads its target as a chunk of its own (bundler: with --splitting --target bun, require() of an ES module is a chunk boundary #40519).Downsides
import()of a marked chunk sees amodule.exportskey. Bun before 1.2.6 returns the old value.reactstatically builds the namespace object at startup: 5.94M to 7.57M instructions, 1,982 B more (React 19.2.0).require("react").self(), still gets the chunk namespace asthis.Notes
How to read the diff. This branch is #47080 plus one commit. Until #47080 lands, the diff against main shows both. The commit of this PR changes
LinkerGraph.rs,postProcessJSChunk.rs,computeCrossChunkDependencies.rs, about 30 lines ofLinkerContext.rsandscanImportsAndExports.rs,bundler_splitting.test.ts, one case ofbundler_cjs2esm.test.tsand three docs passages.The question. #47080 leaves
--splitting --target=bunat the value of main. Two remedies are built and measured.require("react-dom/server"),--splitting --target=bun --productionfind_reachable_fileskeeps a lifted file in the chunk of its caller, as for any CommonJS target. It adds no export and no runtime floor. It gives up the lazy chunk: the table shows the price. Every file that the target imports also gets a lazy__esmwrapper for the whole build, so another entry point that only importsreactpaid 4.4% to 61% more startup instructions (1 to 600 importing files). That form was the head f65c7b7 of bundler: skip the__toCommonJScopy for require() of a lifted CommonJS file #47080..defaultat the call (the route of bundler: read .default of a split require() whose target is CommonJS at link time #41243). The value then belonged to the call, not to the file:require("react").self() === mwasfalse, where main andbun runprinttrue.How it works
"module.exports"fromrequire()of an ES module, when the module has one (src/js/builtins/CommonJS.ts, since module pr 2 #18266 in 1.2.6).dynamic_import_dce/SplitRequireKeepsModuleExportsExportpins that rule for a user-written export. Node does the same (v26.3.0).export { $count as count, exports_react as "module.exports" }.exports_reactis the namespace object that__exportCjsbuilds, with a getter and a setter for each name.js_meta::Flags::needs_module_exports_export. Step 1 ofscan_imports_and_exportssets it for the target of a splitrequire(), whenrequire_returns_namespace(from bundler: skip the__toCommonJScopy for require() of a lifted CommonJS file #47080) is true and the call observes its result: it has no tracked use, or it readsdefaultor__esModule.require("x").a,const { a } = require("x")), the output is the output of main, byte for byte, and the chunk keeps only the names that are read.__commonJSafter step 1 keeps the.defaultroute of bundler: read .default of a split require() whose target is CommonJS at link time #41243.chunk_module_exports_is_namespacetestswrap != Cjsfor that reason (splitting/SplitRequireOfLiftedCommonJSThatIsWrappedAgainLater)..defaultroute is for a chunk that is CommonJS at link time. A lifted chunk cannot use it, because itsdefaultname can be taken by the file.module.exportsitself. The namespace object takes the export name, and the file's own value stays a key of the object (...WithModuleExportsName, and...WithModuleExportsNameAlsoImportedStaticallyfor the cross-chunk import list).debug_assert!ingenerate_entry_point_tail_jsfails when two items of an export clause have one alias.What the export adds
import()of a marked chunk sees amodule.exportskey on its namespace. Node lists that key for a CommonJS module too (count,default,inc,module.exports).bun rundoes not. A lifted user entry point that another entry point requires gets the same export. The metafile does not list the export. It does not list thedefaultthat main adds for animport()of a lifted file either.--compileembeds the current runtime.--production, staticimport { useState }plus a heldrequire("react")that never runs: 5,936,834 to 7,573,994 startup instructions (main a second time: 5,939,094), and the startup chunk grows from 7,135 to 9,117 B.defaultof animport()is. A held call that tree shaking removes still marks its target.Limits that stay
require("react").self()gets the module namespace of the chunk asthis. A mark for every splitrequire()would fix it, and would keep every export of the chunk.import()of the chunk, then readsrequire.cachefor it, then callsrequire(), the call returns the module namespace. Therequire.cacheget trap stores the namespace without a look at the export. The runtime loader is not changed here.ns.__esModuleof animport()of the same chunk turnstrueafter therequire(), as on main.require()cycle through the chunk gives the placeholder{}of the runtime to the inner call, as for any chunk on main (splitting/SplitRequireCycleThroughLiftedCommonJS).require()chunk, on main and with this PR. bake: turn split require off for production builds #46112 (open) turns splitrequire()off there.Open items before this leaves draft
x_default as default, x_default as "module.exports"to the same export clause for--compile. I did not build the two together. If both add the name for one chunk, thedebug_assert!of this PR fails in a debug build. The second one to land has to give the name one owner.wrap != Cjstest covers it. A loop after step 2 that asks the helper again would not depend on that.require_reads_exports_objectrepeats the name pair of the parser'sis_require_marker. One shared helper is better.--compile --splitting. A manual run on the earlier form printedundefined 42 true, alone and with--format=esm --bytecode. Two cases ofbundler_compile_splitting.test.tsthat this PR does not touch time out on a local debug build, so I did not add a case there.__exportCjs, which can move the runtime helpers into a shared chunk.Where the numbers come from. Release builds of a0cc096 (main on that day) and of an earlier form of this change on it, Linux x64,
instructions:ufrom aperf_event_opencounter. The mechanism is the same. Since then the branch got the helper change of #47080 (a lifted re-export of another package), the skip in the cross-chunk import list and the assertion. I did not build a release binary of the final form.try { m = require(spec) }for 33 entry points in six modes: the cells that differ from thenodeorbunoracle go from 113 of 165 on main to 70 with bundler: skip the__toCommonJScopy for require() of a lifted CommonJS file #47080 and 54 with this change on top. The 54 left are the wrapped development builds (46, bundler: skip __toESM for an unwrapped require() of a package that keeps its CommonJS wrapper #46215) andreact-refresh/babel(8).require(): 0 of 956 files differ over 154 builds.js_meta::Flagsgrows from 7 to 8 B for each input file.Tests. 27 cases in
bundler_splitting.test.ts: 26 new and 1 changed. 18 fail on a release build without the change, 8 are controls, 1 is atodo.cjs2esm/KeptRequireOfLiftedCommonJSWithSplitting-SplitRequire, the control that #47080 adds for this route, now expectsundefined ok 42.splitting/SplitRequireOfLiftedCommonJSStaysEsm(from bundler: read .default of a split require() whose target is CommonJS at link time #41243) keeps its assertions (a chunk, a call with no.default). It now also printsm.__esModuleand checks the new export of the chunk.splitting/SplitRequireOfLiftedCommonJSProductionMinifiedis the realreact-dom/index.jsshape with all three minify flags and a productionNODE_ENV.todo:splitting/SplitRequireOfLiftedReExportOfExternal. A liftedmodule.exports = require(<external>)as a split chunk hasexport *inside its wrapper function and fails withUnexpected keyword 'export'. Main has this defect.bundler_cjs2esm.test.ts139 pass, 1 todo.bundler_splitting.test.ts233 pass, 1 skip, 4 todo, 1 fail. The failure issplitting/BarrelCycleKeepsImportOrder: a method names a later file, a 5 s timeout in a case that this PR does not touch.Self-review. The review of the earlier form raised 16 concerns about this route.
minChunkSizecase, a minified production case, the skip in the cross-chunk import list with a case, labels for the controls and two cases that asserted nothing new, the startup cost with a static import, the mark before tree shaking, the assertion on unique aliases, why both mechanisms stay, the docs sentences as user-visible rules.--compilecase, the tracked method call, the helper-chunk shape, the owner of the name with compile: return module.exports from a run-time require() of an embedded CommonJS entry point #40914, one producer for the names that a chunk binds to its namespace object.