[robobun]

Part of #12463

Problem

• In ESM output, require("react") gives a copy with getters and a default key when the package keeps its __commonJS wrapper (development React). A write through an interop helper or import React from "react" throws TypeError: Cannot set property useLayoutEffect of #<Object> which has only a getter.
• Cause: transpose_require (src/js_parser/p.rs) makes the require() an unmarked import * as, and the linker wraps it in __toESM.

Fix

• transpose_require sets ImportRecordFlags::WAS_ORIGINALLY_REQUIRE. The linker does not wrap a record with the flag in __toESM. The value is require_x(): module.exports.
• Its three old readers had no writer and are removed (Notes).
• Verified: test/bundler/bundler_cjs2esm.test.ts (11 new tests fail on main), all 107 files of test/bundler/ on release builds.
• Self-reviewed: 22 concerns raised, 18 addressed (Notes).

Background

• Unwrap list: for ESM output, require() of react, react-dom, scheduler and four more packages, and each require() in their files, becomes import * as ns, so the package tree-shakes.
• A lifted package has exports.x = ... turned into ES exports. Any other package keeps its __commonJS wrapper: require_x() is module.exports.
• Revives bundler: skip __toESM for unwrapped require() that lands on a CJS wrapper #35722, which a stale sweep closed. Weighed: unwrap in the linker, which needs a maintainer's agreement (Notes).

Downsides

• .default of the value is undefined in a development build, as in Node. It was the exports object. A --production build gives the object.
• Not changed (Notes): the value for a lifted package, for --external react, and for a require() in try.

Notes

Repro (React 19.3.1, bun build <file> --target=node --outfile=out.mjs && node out.mjs, main 91e7e3b, also 1.4.2 and 1.3.13):

// helper.js: the output of TypeScript or Babel for `import React from "react"`
function interop(mod) { return mod && mod.__esModule ? mod : { default: mod }; }
const react_1 = interop(require("react"));
react_1.default.useLayoutEffect = react_1.default.useEffect;
// TypeError: Cannot set property useLayoutEffect of #<Object> which has only a getter   (Node: runs)

// held.js
const R = require("react");
console.log(typeof Object.getOwnPropertyDescriptor(R, "version").get, typeof R.default, "default" in R);
// function object true   (Node: undefined undefined false)

The same TypeError for import React from "react"; React.useLayoutEffect = React.useEffect, because react/index.js is module.exports = require('./cjs/react.development.js') and main prints var react_development = __toESM(require_react_development()) there.

Published code hits it. The ES module entry of @legendapp/state 2.1.15 (react-hooks/createObservableHook.mjs) has import React from 'react'; React.useState = function .... With React 18.3.1 a development build of main throws TypeError: Cannot set property useState of #<Object> which has only a getter. With this PR it prints what Node prints.

Which builds keep the wrapper. A default build (development React). A build with NODE_ENV=production in the environment and no --production flag. A package that can be lifted but that the linker wraps again, for example when another file holds its default import as a value (#42569). With --production, React is lifted and the cases above run on main.

What this PR does not change. The value of an unwrapped require() is still not module.exports in these cases. Each prints the same with main and with this PR (React 19.3.1, --target=node):

Case	Bundle	Node
--production, held.js	function object false	undefined undefined false
--external react, held.js	undefined object true	undefined undefined false
--external react, helper.js	TypeError: Cannot assign to property 'useLayoutEffect' of [object Module]	runs
--production, let T; try { T = require("react") } catch {} beside const U = require("react"): T.__esModule, T === U	true false	undefined true
A package whose exports has import and require conditions	loads the import file	loads the require file

const { useState } = require("react") of a package that keeps its wrapper still throws ReferenceError: exports is not defined. #46072 fixes that.

In a TypeScript file, const React = require("react") that nothing reads is dropped with the package, and so is a type-only import React = require("react"). Main does the same. Node and esbuild run the package. Bun runs it for a package that is not on the list, and with --format=cjs.

No switch. This changes the value of an expression, so code can depend on the old value: a read of .default off require("react"), or off the default import of a package that keeps its wrapper, gave the exports object and now gives undefined in a development build. Node, bun run, esbuild and bun build --format=cjs give undefined. #41162 and #42569 changed the value of the default import toward Node in the same way, with no switch.

The three old readers of the flag, removed here:

• src/js_parser/scan/scan_imports.rs, in the unused-import check for TypeScript (// require statements cannot be stripped). With a writer it keeps the two statements of the paragraph above. I built that: 991 tests of nine bundler files pass with no change. It also changes the output of TypeScript files. React is bundled where main drops it, and an unused require() of a listed package that is not installed becomes Could not resolve. That is not the change this PR describes, so the reader goes and the output stays as on main.
• src/js_parser/scan/scan_imports.rs, in the branch that does not bundle. A require() is unwrapped only when bundling.
• src/js_printer/lib.rs. It needs a namespace alias that is not a property access and that has a record index. The parser makes those only for hot module reloading, which does not unwrap.

The flag is read at one site, the __toESM decision for a target that has a wrapper (scanImportsAndExports.rs). Every other test of ImportKind::Require still sees an import: the resolve condition, the order of evaluation, whether the target is wrapped.

Other open PRs on these lines

• Remove dead code from the bundler, the resolver, bun_css, the Valkey client, and six smaller areas #45723 deletes the flag and the three readers as dead code. If this PR merges first, Remove dead code from the bundler, the resolver, bun_css, the Valkey client, and six smaller areas #45723 keeps the flag. If Remove dead code from the bundler, the resolver, bun_css, the Valkey client, and six smaller areas #45723 merges first, this PR adds the flag again.
• bundler: read the import namespace for a destructured require() of an unwrapped package #46072 (draft) owns the destructured require(). No line of this PR overlaps it.
• bundler: give import * of a lifted CommonJS module its own namespace object #41820 has its own test for "this import * was a require()" (import_star_was_require_call). It can read the flag.

Builds with no write (release builds of main and of this PR, 33 entries x 7 flag sets x React 18.3.1 and 19.3.1):

	builds
byte-identical	263
smaller, by 8 to 1,223 B	183
larger	0
fail on main and on this PR	16

The run output changes in 36 builds. In each one this PR prints what Node prints for the unbundled entry: the value has no default key and no getters, require("react") === require("react") through a function, and the write through the default import runs.

Existing tests that change, each pinned the copy:

• cjs2esm/UnwrappedModuleRequireAssigned expected __toESM( in the output.
• cjs2esm/ReactSpecificUnwrappingTargetIsCommonJS expected typeof ReactDOM to be object for a package whose module.exports is a function.
• splitting/SplitRequireOfRewrappedLiftedCommonJS#41236 called m.default() on that package.
• npm/ReactSSR: three source map columns move by 8, and the bundle is 221,992 B (main 222,012 B).

Measured (release builds, main 91e7e3b against this PR)

• Instructions for a build of React 19.3.1 with react-dom/server: 101.7 M against 101.8 M, with a spread of 0.7% between runs. For a file of 100,000 React.count + o[k] reads: 1,265.4 M against 1,266.0 M (spread 0.05%).
• Allocator calls for the first build: 95,319 against 95,348 (lowest of three runs).
• Release .text: 1,024 B smaller.

The larger design. A design review ranked one design above this one: keep require() of a listed package an ImportKind::Require record and move the unwrap decision from the parser to linker step 1. It needs no flag, and it would also close the cases in the table above. It moves every build that uses a listed package to new linker code (about 2,200 lines by estimate, not built). It has to reach the size of today's unwrap: for React 19.3.1 --production, const R = require("react"); R.useState is 910 B, and the same read through a real require record is 1,679 B when the linker can track it and 10,131 B when it cannot. REVIEW.md asks for a maintainer's agreement before a change of that kind. The question is already open on #45343 ("bind at link time" is its third option), so I do not ask it again here. This PR does not depend on the answer: it is the one-pass form, and the flag is also what a link-time design would read.

Self-review. Not taken:

• A test helper that takes each expected line from a run of the unbundled entry. This file pins literal output in every test. I checked each literal against Node.
• A tracking issue for the cases in "What this PR does not change". They are listed above with their output.
• Keeping the wrapper whenever the value of a require() is held (two concerns). That belongs to bundler: keep the CommonJS wrapper of a package that a file writes to through require() #46216, the PR on top of this one, and there it would take the wrapper of React for each file that passes require("react") to an interop helper.

[robobun]

^{Updated 11:05 AM PT - Oct 20th, 2026}

@robobun, your commit 2f0cee8 is building: #129571

[robobun]

Status

Reproduced on main (91e7e3b) with React 19.3.1. The file helper.js in the Notes of the PR body holds require("react") through an interop helper and writes react_1.default.useLayoutEffect. bun build helper.js --target=node --outfile=out.mjs builds it, and node out.mjs throws TypeError: Cannot set property useLayoutEffect of #<Object> which has only a getter. Node runs the file. The 11 new tests in test/bundler/bundler_cjs2esm.test.ts fail on main and pass with this branch.

PR: #46215

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 9d12f0cb-4fbf-416e-96fc-e6cb10cf3f10

📥 Commits

Reviewing files that changed from the base of the PR and between ea6a2c7 and 2f0cee8.

📒 Files selected for processing: 8

• src/ast/import_record.rs
• src/bundler/linker_context/scanImportsAndExports.rs
• src/js_parser/p.rs
• src/js_parser/scan/scan_imports.rs
• src/js_printer/lib.rs
• test/bundler/bundler_cjs2esm.test.ts
• test/bundler/bundler_npm.test.ts
• test/bundler/bundler_splitting.test.ts

💤 Files with no reviewable changes: 1

• src/js_printer/lib.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

The parser now marks imports converted from require() calls. Bundling paths use that marker to avoid ESM wrapping for those imports. Tests cover direct module.exports values across package shapes, entry points, and code splitting.

Changes

CommonJS require exports

Layer / File(s)	Summary
Mark require-originated imports src/ast/import_record.rs, src/js_parser/p.rs	Import records created from require() calls now retain the WAS_ORIGINALLY_REQUIRE flag. The flag documentation describes the retained-wrapper case.
Handle require-originated imports src/js_parser/scan/scan_imports.rs, src/bundler/linker_context/scanImportsAndExports.rs, src/js_printer/lib.rs	Import scanning no longer preserves unused require-originated imports or creates their namespace aliases. The linker excludes flagged records from __toESM wrapping. The printer removes the special disabled-import branch for these records.
Validate direct CommonJS exports test/bundler/bundler_cjs2esm.test.ts, test/bundler/bundler_splitting.test.ts, test/bundler/bundler_npm.test.ts	Tests check direct module.exports values, identity and mutations, redirected and wrapped package shapes, entry points, and split require behavior. ReactSSR snapshot offsets and output size are updated.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 2f0ce

No concrete issue remains that would prevent merging after normal validation.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary bundler change: skipping __toESM for unwrapped require() calls that retain a CommonJS wrapper.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description clearly explains the problem, root cause, fix, scope, known limitations, and verification results. It does not use the exact template headings, but it provides equivalent and substanti…

Full details: Description check

Explanation

The description clearly explains the problem, root cause, fix, scope, known limitations, and verification results. It does not use the exact template headings, but it provides equivalent and substantially more complete information.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the rewrite of splitting/SplitRequireOfRewrappedLiftedCommonJS#41236 in test/bundler/bundler_splitting.test.ts: the old m.default() assertion pinned exactly the __toESM copy this PR removes, and the cross-chunk regression it guards (import.meta.require("./chunk").default in main.js plus the module running) is still asserted, so it is an expected-output update rather than a weakened test.

Extended reasoning...

The change sets the previously never-written WAS_ORIGINALLY_REQUIRE flag in the parser's require-to-namespace-import path, gates the linker's __toESM insertion on it, and deletes three prior readers in scan_imports.rs and js_printer/lib.rs; it touches no security-sensitive surface. The inline findings cover the --production inconsistency and two pre-existing gaps, so a human still needs to weigh those; the mutated splitting test was examined and found to be a legitimate update of the behavior under change.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 src/js_printer/lib.rs — Users with const { useState } = require("react") in an ESM bundle of development React still crash at load with ReferenceError: exports is not defined; this PR changes what is printed there but not the crash. The parser keeps the E::RequireString with unwrapped_id for an object binding (visit/mod.rs:400-408 only rewrites identifier bindings), and lib.rs:2869-2873 prints the bare exports_ref of the wrapped target without calling its wrapper. With WRAP_WITH_TO_ESM now cleared at scanImportsAndExports.rs:1237 the output is the unbound exports symbol alone. Fix: when was_unwrapped_require and the target keeps a __commonJS wrapper, print the wrapper call (require_react()), the same value :1237 now chooses for the identifier form.

  Why this was flagged

  Trigger: an ESM build (ParseTask.rs:2511) containing const { useState } = require("react"), with React kept in its __commonJS wrapper (default development build). p.rs:1725-1746 returns E::RequireString { unwrapped_id } because the require is immediately assigned to a decl; visit/mod.rs:397-408 converts only BIdentifier bindings into the namespace ref, so the BObject form survives to the printer. LinkerContext.rs:2548-2562 returns exports_ref and was_unwrapped_require = true because the react file carries FORCE_CJS_TO_ESM (p.rs:9369). lib.rs:2869-2873 then prints just that symbol. For a wrapped CJS file the exports symbol is the wrapper closure's parameter, unbound at module top level, so the bundle throws ReferenceError: exports is not defined on load. Before this PR the same site printed __toESM(exports) and threw the same error; the PR gates the __toESM decision on the new flag at :1237 but leaves this sibling print path still emitting a value that is never module.exports.

  Verification: pre-existing; acknowledged in diff. mod.rs:397-408 rewrites the namespace only for BData::BIdentifier, so the BObject form keeps the ERequireString. lib.rs:2869-2873 prints only meta.exports_ref, the exports parameter of the __commonJS wrapper, with no require_react() call. Both base and PR throw ReferenceError: exports is not defined at load by the same route.

[robobun]

On the additional finding in the review above, const { useState } = require("react") of development React: confirmed. ReferenceError: exports is not defined is on main. This PR changes the text that is printed there and not the crash. The body names it below the table "What this PR does not change".

The fix is not in this PR on purpose. #46072 is open for this crash. It removes the was_unwrapped_require parameter of require_or_import_meta_for_source, so a second fix in the same function would collide with it. #46216, the PR on top of this one, needs the call of the wrapper for its own tests and has that one hunk. With it, the destructured require("react") of development React 18.3.1 and 19.3.1 runs. If #46072 merges first, the hunk goes.

The two inline comments have replies in their threads.