Bun.Transpiler: default autoImportJSX to true for the automatic runtime - #35557
Bun.Transpiler: default autoImportJSX to true for the automatic runtime#35557robobun wants to merge 15 commits into
Conversation
WalkthroughChanges
Automatic JSX Runtime Imports
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Reproduced with All review feedback addressed. Coverage spans the zero-arg constructor, CI: |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I checked the other Bun.Transpiler JSX assertions in transpiler.test.js for breakage from the new import prefix — the remaining ones either use .toContain (lines 2168-2177, 2295-2297), have no JSX elements in the input (2291-2294), or are under it.todo (2300-2402), so the three autoImportJSX: false additions are sufficient. react-compiler.test.ts uses loader: "js" and is unaffected.
Extended reasoning...
Checked whether the three existing tests that gained autoImportJSX: false were the complete set of exact-output JSX assertions that would break under the new default. Grepped test/ for jsxDEV_7x81h0kn and new Bun.Transpiler with JSX loaders; the remaining hits in transpiler.test.js either assert via .toContain (import prefix doesn't matter), transpile inputs without JSX elements (no auto-import emitted), or sit under it.todo. react-compiler.test.ts re-parses bundler output with loader: "js", so JSX auto-import doesn't apply. No missed test-suite breakage found.
|
Updated 9:19 PM PT - Aug 6th, 2026
✅ @robobun, your commit 2f2125e73a65cebef62c32c32acd3d114ac67e09 passed in 🧪 To try this PR locally: bunx bun-pr 35557That installs a local version of the PR into your bun-35557 --bun |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
All four items from my earlier passes are addressed — the .transform()/.scan() coverage, the mdx output block, the regression-test comment, and the multi-line comment on auto_import_jsx: true are all fixed in dbfb1e8. I'm not auto-approving only because this flips a documented public-API default (and deliberately changes .scan() output on JSX to include the synthetic runtime import), which is the kind of user-visible behavior shift a maintainer should sign off on.
What was reviewed:
- The one-line
Config::default()change and its flow intotranspiler.options.auto_import_jsxvia the constructor — no other codepath readsconfig.runtime.auto_import_jsx. - Test coverage now spans no-arg/dev/prod/
jsxImportSource/key-after-spreadcreateElement/classic-runtime/opt-out/.scan()/async.transform(); the three pre-existing tests that asserted the raw output correctly opt out withautoImportJSX: falserather than being weakened. .scanImports()goes through the lexer-onlyScanPassResultpath (notTranspiler::parse), so it does not pick up the synthetic import — checked and that's unchanged.
Extended reasoning...
Overview
One-line runtime change in src/runtime/api/JSTranspiler.rs (Config::default() now sets runtime.auto_import_jsx = true), plus a JSDoc addition in bun.d.ts, an mdx doc update, and ~85 lines of new tests in transpiler.test.js. Fixes #7499 where Bun.Transpiler under the automatic JSX runtime emitted jsxDEV_7x81h0kn(...) with no import for it, yielding unrunnable output.
Security risks
None. No untrusted input handling, no auth/crypto, no memory-safety changes. The Rust diff is a single boolean field initializer inside a Default impl.
Level of scrutiny
Medium. The mechanical change is trivial and clearly correct — the previous default produced code that referenced an undeclared identifier, and Bun.build, esbuild, tsc, swc, and Babel all default this on for the automatic runtime. But it is a public API default flip: three existing tests had to add autoImportJSX: false to keep passing, and .scan() on JSX now returns [{path:"react/jsx-dev-runtime", kind:"import-statement"}] where it used to return []. That .scan() change was made deliberate and tested per my earlier feedback, but whether the synthetic import should surface there (vs. being filtered like IS_INTERNAL records) is a product call a maintainer should confirm.
Other factors
All four points from my prior reviews (2026-07-25T05:18 and 07:01) are addressed as of dbfb1e8: async .transform() and .scan() coverage added, mdx example output now shows the import line, the regression-test comment is trimmed to just the issue URL, and the multi-line comment above auto_import_jsx: true is dropped (comment-cop is satisfied). I also verified .scanImports() is unaffected — it uses the lexer-only bun_bundler::cache::JavaScript::scan() path against scan_pass_result, not Transpiler::parse(), so auto_import_jsx never reaches it. The no-arg-constructor test added in dbfb1e8 closes the last gap I'd have asked about.
scanImports() on JSX unconditionally reported two require-call records (jsx-dev-runtime plus the classic source) regardless of the configured JSX runtime or the autoImportJSX flag, diverging from scan() which reports the single import-statement the transform actually emits. _scan_imports now uses the same gate as the full-parse auto-import path (jsx.parse && auto_import_jsx && runtime == Automatic), emits ImportKind::Stmt, and drops the phantom classic record. The JSTranspiler scanImports entry point now plumbs auto_import_jsx into the parser options so the gate has the user's value.
eb79cf4 to
c3c0717
Compare
…ect the runtime import
…n NODE_ENV in the pragma test rows
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bun-types/bun.d.ts`:
- Around line 2466-2469: Update the automatic JSX runtime documentation near the
JSX transform option to show that jsxDEV is imported from
"<jsxImportSource>/jsx-dev-runtime", including the configured import source such
as preact/jsx-dev-runtime, rather than describing it as a bare jsx-dev-runtime
path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ea9a67a-5bbc-44dd-b491-5b01baa923d0
📒 Files selected for processing (5)
docs/runtime/transpiler.mdxpackages/bun-types/bun.d.tssrc/js_parser/parse/parse_entry.rssrc/runtime/api/JSTranspiler.rstest/bundler/transpiler/transpiler.test.js
There was a problem hiding this comment.
All prior feedback has been addressed and no new issues turned up this round. Deferring to a human for the final call since this flips a user-facing Bun.Transpiler default and changes what .scanImports() reports for JSX inputs.
What was reviewed
- The
auto_import_jsx: truedefault inJSTranspiler.rsand its propagation into thescan_importsparser opts. - The
_scan_importsgate rewrite inparse_entry.rs: pragma application now mirrorsprepare_for_visit_pass(bothruntimeanddevelopment), the comment sits on the right block, and the record kind isStmt. - Test matrix pins
NODE_ENVviadefineon every row that asserts a dev/prod runtime path; the key-after-spread.scanImports()divergence is documented in the PR body as an accepted inaccuracy.
Extended reasoning...
Overview
The PR flips Bun.Transpiler's autoImportJSX default from false to true so that automatic-runtime JSX output includes the react/jsx-runtime (or dev) import it depends on — fixing #7499 where the default output referenced an undeclared jsxDEV_... binding. It touches five files: the Config::default() in JSTranspiler.rs (+1 line to propagate the flag into scan_imports's parser opts), the _scan_imports JSX gate in parse_entry.rs (rewritten to mirror the full-parse gate: honors auto_import_jsx, checks runtime == Automatic, applies the @jsxRuntime pragma, and emits a single ImportKind::Stmt record instead of two Require records), ~130 lines of new tests in transpiler.test.js, and doc/type updates.
Security risks
None. No untrusted-input parsing changes, no auth/crypto/permissions surface. The parser change only refines which import records are synthesized for JSX in the scan pass.
Level of scrutiny
Medium-high. The core fix is a two-line default flip that is clearly correct — the previous default produced code that throws ReferenceError at runtime, and Bun.build/esbuild/tsc/swc/Babel all default this on. But it is still a user-facing behavior change: (a) .transformSync()/.transform() output now includes an import statement it didn't before, and (b) .scanImports() on JSX input now returns one import-statement record for the runtime subpath instead of two require-call records regardless of config. Any tooling that consumed either output verbatim will observe the change. Per the repo's "API design" guidance for changing user-facing surface, a maintainer should sign off.
Other factors
This PR went through six review iterations. Earlier rounds surfaced and fixed: NODE_ENV-dependent test flakiness (now pinned via define), the @jsxRuntime pragma not being consulted in scan mode (now applied, including the development half of RUNTIME_MAP), a misplaced code comment, and bug-history narration in test comments. The one consciously-accepted gap — .scanImports() reports the runtime subpath rather than the bare package for the deprecated key-after-spread createElement fallback — is documented in the PR description with the rationale (merge_jsx doesn't propagate package_name; scanImports is spec'd as marginally less accurate). Three pre-existing tests that asserted on the un-prefixed output now pass autoImportJSX: false explicitly. The evidence block shows the new tests fail on main and pass on the PR under both ASAN-debug and release.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/runtime/transpiler.mdx`:
- Around line 220-223: Update the autoImportJSX documentation to cover all
automatic runtime import forms: production imports such as jsx from the
configured jsxImportSource’s runtime module, development imports of jsxDEV from
its dev-runtime module, and how custom jsxImportSource changes those paths.
In `@src/js_parser/parse/parse_entry.rs`:
- Around line 488-510: Update the JSX handling in _scan_imports to apply the
`@jsxImportSource` pragma via jsx_import_source() before constructing the
automatic runtime import record, matching prepare_for_visit_pass behavior.
Ensure both .scan() and .scanImports() resolve the pragma source instead of
defaulting to react/jsx-runtime, and add regression coverage for each API.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d98203a-a69b-4d71-8b7a-d71542ff75b2
📒 Files selected for processing (5)
docs/runtime/transpiler.mdxpackages/bun-types/bun.d.tssrc/js_parser/parse/parse_entry.rssrc/runtime/api/JSTranspiler.rstest/bundler/transpiler/transpiler.test.js
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
test/bundler/transpiler/transpiler.test.js:2594— The merge from main (57c7ff1) pulled in a fourth exact-match JSX test — "JSX tag names containing '-' or ':' are string tags regardless of case" at transpiler.test.js:2614-2635 — that constructsnew Bun.Transpiler({ loader: "jsx", define: {...} })withoutautoImportJSX: falseand does six.toBe()assertions expecting un-prefixedexport var foo = jsxDEV_7x81h0kn(...). With this PR's default flip, each output is now prefixed withimport { jsxDEV as ... } from "react/jsx-dev-runtime";\nand every assertion fails. AddautoImportJSX: falseto the options at line 2621, matching the three sibling fixes already in this diff.Extended reasoning...
What the issue is
This PR flips
Bun.Transpiler'sautoImportJSXdefault fromfalsetotrue(src/runtime/api/JSTranspiler.rs:99). To keep the existing suite passing, the diff addsautoImportJSX: falseto the three pre-existing tests that assert exact un-prefixed JSX output via.toBe()— at lines 2207, 2452, and 2594 oftest/bundler/transpiler/transpiler.test.js. But a fourth such test now exists at lines 2614–2635, and it does not haveautoImportJSX: false.How it got here
The test "JSX tag names containing '-' or ':' are string tags regardless of case" was added on
mainin commit 2c5c312 (PR #37052, 2026-08-06) and pulled into this branch by merge commit 57c7ff1 — the most recent commit on the branch. This PR's evidence block shows the last gate run (iteration 6) was againsta1f37750e, which predates the merge. So the gate never exercised this test with the flipped default, and the three-site sweep in this diff was complete against the pre-merge tree but is now one short.The code path
The test (transpiler.test.js:2617-2622) constructs:
const bun = new Bun.Transpiler({ loader: "jsx", define: { "process.env.NODE_ENV": JSON.stringify("development"), }, });
With no explicit
autoImportJSX,Config::default()now setsauto_import_jsx: true(JSTranspiler.rs:99), which propagates totranspiler.options.auto_import_jsxand then into the parser'soptions.features.auto_import_jsx. The full-parse auto-import gate inparse_entry.rs(p.options.jsx.parse && p.options.features.auto_import_jsx && p.options.jsx.runtime == Automatic) passes, sogenerate_import_stmt("react/jsx-dev-runtime", ["jsxDEV"], ...)prepends the import to the output.Step-by-step proof
Take the first loop iteration,
tag = "Foo-Bar",expected = '"Foo-Bar"':- Input:
export var foo = <Foo-Bar />. transformSyncruns the automatic JSX transform in dev mode → the JSX becomesjsxDEV_7x81h0kn("Foo-Bar", {}, undefined, false, undefined, this), andjsxDEVis recorded as used.- With
auto_import_jsx = true(the new default), the post-visit pass emitsimport { jsxDEV as jsxDEV_7x81h0kn } from "react/jsx-dev-runtime";as the first statement. - Actual output:
import { jsxDEV as jsxDEV_7x81h0kn } from "react/jsx-dev-runtime"; export var foo = jsxDEV_7x81h0kn("Foo-Bar", {}, undefined, false, undefined, this); - The assertion at line 2631 is:
The expected string has no import prefix, so
expect(bun.transformSync(`export var foo = <${tag} />`)).toBe( `export var foo = jsxDEV_7x81h0kn(${expected}, {}, undefined, false, undefined, this);\n`, );
.toBe()fails. The same happens for all six[tag, expected]pairs.
Why existing code doesn't prevent it
The three
autoImportJSX: falseadditions in this diff (2207, 2452, 2594) each patch a specific transpiler-instance construction site; there is no shared helper. The new test at 2617 constructs its ownBun.Transpilerlocally, so nothing in this PR's diff touches it. Per REVIEW.md: "When changing output/defaults/messages, grep the suite for assertions on the old behavior and update them in the same PR" — the merge introduced a new site matching the old-behavior assertion pattern after that grep was done.Impact
Concrete CI failure:
bun bd test test/bundler/transpiler/transpiler.test.jswill report 6 failed assertions in this test, blocking merge.Fix
Add
autoImportJSX: falseto the transpiler options at line 2621, exactly matching the three sibling fixes already in this diff:const bun = new Bun.Transpiler({ loader: "jsx", define: { "process.env.NODE_ENV": JSON.stringify("development"), }, autoImportJSX: false, });
- Input:
There was a problem hiding this comment.
No new issues found — all eight rounds of prior feedback are addressed and resolved. Deferring to a maintainer because this flips a documented Bun.Transpiler default (autoImportJSX false→true) and reshapes .scan()/.scanImports() output for JSX input, which is a user-visible API behavior change worth a human sign-off even though the old default produced unrunnable output.
What was reviewed
_scan_importspragma handling (2f2125e) mirrorsprepare_for_visit_passat p.rs:2781-2795 field-for-field, includingdevelopmentandpackage_name.- Confirmed
_scan_importsis only reachable viaBun.Transpiler.scanImports()(bundler proper uses the full-parse path), so the import-record shape change is scoped to that one API. - New test matrix pins
NODE_ENVviadefine/tsconfig on every row that asserts a specific dev/prod runtime path — the ambient-env fragility from earlier rounds is not reintroduced.
Extended reasoning...
Overview
Flips the Bun.Transpiler default for autoImportJSX from false to true, so the automatic JSX runtime emits the import { jsx… } from '<src>/jsx-runtime' statement alongside the transformed calls (fixes #7499 — the previous default produced output referencing an undeclared hashed identifier). Also rewrites the JSX-import block in _scan_imports (parse_entry.rs) to mirror prepare_for_visit_pass: it now applies @jsxImportSource and @jsxRuntime pragmas, gates on auto_import_jsx && runtime == Automatic, and emits a single ImportKind::Stmt record instead of two Require records. JSTranspiler.rs wires the config default and threads auto_import_jsx into the scanImports parser opts. Docs (transpiler.mdx) and bun.d.ts JSDoc updated; ~140 lines of tests added.
Security risks
None. No untrusted-input parsing changes, no allocation-size arithmetic, no FFI/lifetime changes. The arena-copy of import_source in _scan_imports was already there; only the gate and record kind changed.
Level of scrutiny
Medium-high. The core fix (default flip in Config::default() + one line in scan_imports) is trivial, and the _scan_imports rewrite is a faithful subset of the reference in prepare_for_visit_pass. But this is a user-visible behavioral change to three documented APIs (transformSync/transform output now prepends an import; .scan().imports and .scanImports() now report the synthetic JSX runtime import where they previously did not / reported different records). The rationale is strong — Bun.build, esbuild, tsc, swc, and Babel all default this on, and the old output was dead-on-arrival — but flipping a documented default is a product decision a maintainer should confirm.
Other factors
- This PR has been through eight review iterations; every prior inline finding from me (variant-matrix coverage,
.scan()behavior,@jsxRuntimepragma in scan mode,RUNTIME_MAP.developmentfield, ambient-NODE_ENVtest fragility, comment placement) and from CodeRabbit (@jsxImportSourcepragma in scan mode, doc wording) is marked resolved and verified against the current diff. - Test coverage is thorough: zero-arg constructor, dev/prod, sync/async, custom
jsxImportSource, key-after-spreadcreateElementfallback, classic-runtime no-op, explicit opt-out, and a 10-row.scanImports()↔.scan()agreement matrix including three pragma-override cases and the@jsxImportSourcepragma. - Three pre-existing tests that asserted exact un-prefixed output were updated to pass
autoImportJSX: falserather than having their assertions weakened. - Blast radius of the
_scan_importsrewrite is limited: the only caller ofParser::scan_importsisbun_bundler::cache::JavaScript::scan, and that is only invoked fromJSTranspiler::scan_imports— the bundler's own parse path is unaffected.
2 participants
What does this PR do?
Fixes #7499.
Bun.Transpilerwith the automatic JSX runtime replaces<div/>with a call through a generated binding such asjsxDEV_7x81h0kn(...). That binding only exists when the matchingimport { jsxDEV as jsxDEV_... } from ".../jsx-dev-runtime"is emitted alongside it. The transpiler defaultedautoImportJSXtofalse, so the default output referenced a name that was never declared and threw at runtime:Repro
Before:
After:
Why is this fix correct?
The whole point of the automatic JSX runtime is that the compiler inserts the import for you.
Bun.buildalready defaultsauto_import_jsxtotrue(seeBundleOptionsinsrc/bundler/options.rs), as do esbuild,tsc,swc, and Babel when targeting the automatic runtime. Without the import the output is dead on arrival: the generated identifier carries a collision-avoidance hash suffix, so there is no name a user could import manually to make it work.The classic runtime (
React.createElement) is unaffected. Users who want the raw output without the import can still passautoImportJSX: false..scan()/.scanImports().scan()on JSX input now reports thereact/jsx-runtime(orreact/jsx-dev-runtime) dependency, since the emitted code actually imports it..scanImports()previously reported tworequire-callrecords (<src>/jsx-dev-runtimeand the classic source) for any JSX regardless of the configured runtime orautoImportJSX. It now mirrors the full-parse gate: a singleimport-statementrecord for the runtime subpath, only under the automatic runtime withautoImportJSXenabled, so.scanImports()and.scan()agree on the injected import. The scan pass still has no per-symbol use counts, so the deprecated key-after-spreadcreateElementfallback (which imports from the bare package instead of the runtime subpath) remains a known.scanImports()inaccuracy.How did you verify your code works?
Added coverage in
test/bundler/transpiler/transpiler.test.jsfor the default now emitting the import across the zero-arg constructor,.transformSync()in dev and production (react-jsx) mode, async.transform(), a customjsxImportSource, and thecreateElementfallback after a key-after-spread. Also asserts the classic runtime is unchanged,autoImportJSX: falsestill suppresses the import, and.scan()/.scanImports()both surface the runtime import across the automatic-runtime variants while staying empty for the classic runtime, the opt-out, and sources with no JSX.The three existing tests that asserted on the exact un-prefixed output now set
autoImportJSX: falseexplicitly. Updated the sample output indocs/runtime/transpiler.mdxand addedautoImportJSXto its reference section and to theTranspilerOptionsJSDoc inbun.d.ts.[review] gate passed · iteration 8 · 5 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 5 passed · 2 rejected · iteration 8
evidence per changed file