Bun.Transpiler: default autoImportJSX to true for the automatic JSX runtime - #35582
Bun.Transpiler: default autoImportJSX to true for the automatic JSX runtime#35582robobun wants to merge 1 commit into
Conversation
…untime With the automatic JSX runtime (the default), Bun.Transpiler rewrites <div /> to a call against a generated symbol (jsxDEV_7x81h0kn, etc.) but only emits the matching import when autoImportJSX is true. The option previously defaulted to false, so the default transpiler output referenced an undefined identifier and could not be run or re-bundled. This flips the Bun.Transpiler default to true. The bundler and the module loader already enable it, so this only changes the standalone API. The classic runtime and an explicit autoImportJSX: false are unchanged. Fixes #7499.
WalkthroughChangesAutomatic JSX runtime
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 2465-2472: Update the documentation comment for the automatic JSX
runtime option to mention that development transforms import from
<jsxImportSource>/jsx-dev-runtime, using react/jsx-dev-runtime as an example,
while retaining the existing /jsx-runtime path for other automatic transforms.
In `@test/bundler/transpiler/transpiler.test.js`:
- Around line 2158-2161: In the regression test around the issue 7499 URL,
remove the explanatory comments describing automatic JSX runtime behavior and
retain only the issue URL comment. Apply the same cleanup to the corresponding
repeated regression-test comment locations, preserving the test code unchanged.
- Around line 2173-2189: Update the JSX runtime tests around the preact,
classic, and autoImportJSX opt-out cases to run for both “jsx” and “tsx”
loaders, following the repository’s existing matrix-test convention. Preserve
each case’s current assertion while parameterizing only the loader so both
transformation paths are covered.
🪄 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: facbe5d1-88ed-4b01-b57a-3877ab20cc9c
📒 Files selected for processing (3)
packages/bun-types/bun.d.tssrc/runtime/api/JSTranspiler.rstest/bundler/transpiler/transpiler.test.js
| /** | ||
| * When the automatic JSX runtime is active, prepend the | ||
| * `import { jsx, ... } from "<jsxImportSource>/jsx-runtime"` statement that | ||
| * binds the generated `jsx`/`jsxs`/`jsxDEV`/`Fragment` calls. | ||
| * | ||
| * Has no effect when the classic runtime (`jsx: "react"`) is in use. | ||
| * | ||
| * @default true |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the development runtime import path.
Automatic development transforms import <jsxImportSource>/jsx-dev-runtime—for example, the test expects react/jsx-dev-runtime on Line 2166—while this documentation only names /jsx-runtime despite mentioning jsxDEV.
Proposed documentation fix
- * `import { jsx, ... } from "<jsxImportSource>/jsx-runtime"` statement that
+ * `import { jsx, ... } from "<jsxImportSource>/jsx-runtime"` (or
+ * `"<jsxImportSource>/jsx-dev-runtime"` in development) that📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * When the automatic JSX runtime is active, prepend the | |
| * `import { jsx, ... } from "<jsxImportSource>/jsx-runtime"` statement that | |
| * binds the generated `jsx`/`jsxs`/`jsxDEV`/`Fragment` calls. | |
| * | |
| * Has no effect when the classic runtime (`jsx: "react"`) is in use. | |
| * | |
| * @default true | |
| /** | |
| * When the automatic JSX runtime is active, prepend the | |
| * `import { jsx, ... } from "<jsxImportSource>/jsx-runtime"` (or | |
| * `"<jsxImportSource>/jsx-dev-runtime"` in development) statement that | |
| * binds the generated `jsx`/`jsxs`/`jsxDEV`/`Fragment` calls. | |
| * | |
| * Has no effect when the classic runtime (`jsx: "react"`) is in use. | |
| * | |
| * `@default` true |
🤖 Prompt for 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.
In `@packages/bun-types/bun.d.ts` around lines 2465 - 2472, Update the
documentation comment for the automatic JSX runtime option to mention that
development transforms import from <jsxImportSource>/jsx-dev-runtime, using
react/jsx-dev-runtime as an example, while retaining the existing /jsx-runtime
path for other automatic transforms.
| // https://github.com/oven-sh/bun/issues/7499 | ||
| // The automatic JSX runtime emits calls to generated symbols (jsxDEV_7x81h0kn | ||
| // etc.). Bun.Transpiler previously defaulted autoImportJSX to false, so those | ||
| // calls were left undefined in the output. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep regression-test comments to the issue URL only.
The repository guideline requires regression tests to contain exactly the issue URL comment. Keep the URL at Line 2158 and remove the explanatory comments in this test.
Proposed cleanup
// https://github.com/oven-sh/bun/issues/7499
-// The automatic JSX runtime emits calls to generated symbols ...
-// ...
-// Every generated JSX symbol that is called/used must be bound by the import.
-// tsconfig jsxImportSource is honored by the default-on auto-import.
-// The classic runtime has no auto-import and must stay import-free.
-// autoImportJSX: false opts out of the import.Also applies to: 2167-2167, 2173-2173, 2180-2180, 2187-2187
🤖 Prompt for 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.
In `@test/bundler/transpiler/transpiler.test.js` around lines 2158 - 2161, In the
regression test around the issue 7499 URL, remove the explanatory comments
describing automatic JSX runtime behavior and retain only the issue URL comment.
Apply the same cleanup to the corresponding repeated regression-test comment
locations, preserving the test code unchanged.
Source: Coding guidelines
| // tsconfig jsxImportSource is honored by the default-on auto-import. | ||
| const preact = new Bun.Transpiler({ | ||
| loader: "tsx", | ||
| tsconfig: { compilerOptions: { jsx: "react-jsx", jsxImportSource: "preact" } }, | ||
| }).transformSync("export default <div />"); | ||
| expect(preact).toContain('from "preact/jsx-runtime"'); | ||
|
|
||
| // The classic runtime has no auto-import and must stay import-free. | ||
| const classic = new Bun.Transpiler({ | ||
| loader: "tsx", | ||
| tsconfig: { compilerOptions: { jsx: "react" } }, | ||
| }).transformSync("export default <div />"); | ||
| expect(classic).not.toContain("import "); | ||
|
|
||
| // autoImportJSX: false opts out of the import. | ||
| const optOut = new Bun.Transpiler({ loader: "tsx", autoImportJSX: false }).transformSync("export default <div />"); | ||
| expect(optOut).not.toContain("import "); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover both jsx and tsx for every changed behavior.
The default-import assertion iterates over both loaders, but the jsxImportSource, classic-runtime, and autoImportJSX: false cases only exercise tsx. A regression in the corresponding jsx path could therefore pass unnoticed. Parameterize these cases across both loaders, preferably using the repository’s matrix-test convention.
🤖 Prompt for 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.
In `@test/bundler/transpiler/transpiler.test.js` around lines 2173 - 2189, Update
the JSX runtime tests around the preact, classic, and autoImportJSX opt-out
cases to run for both “jsx” and “tsx” loaders, following the repository’s
existing matrix-test convention. Preserve each case’s current assertion while
parameterizing only the loader so both transformation paths are covered.
Source: Coding guidelines
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Duplicate of #35557, which applies the same default change and already has green CI. Closing in favor of that one. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| log: bun_ast::Log::default(), // overwritten at construction | ||
| runtime: Runtime::Features { | ||
| top_level_await: true, | ||
| auto_import_jsx: true, |
There was a problem hiding this comment.
🟡 The .transformSync() example output in docs/runtime/transpiler.mdx (lines 36-43) still shows the bare jsxDEV_7x81h0kn(...) call with no defining import — i.e. the pre-PR output. Now that autoImportJSX defaults to true, the real output for that exact snippet begins with import { jsxDEV as jsxDEV_7x81h0kn } from "react/jsx-dev-runtime";. Worth refreshing the doc block in this PR since it's the one place in the docs that shows what that mangled identifier is, and leaving it undefined-looking perpetuates the #7499 confusion.
Extended reasoning...
What's stale
docs/runtime/transpiler.mdx documents .transformSync() with this exact call shape:
const transpiler = new Bun.Transpiler({ loader: 'tsx' });
const code = `
import * as whatever from "./whatever.ts"
export function Home(props: {title: string}){
return <p>{props.title}</p>;
}`;
const result = transpiler.transformSync(code);and shows the output (lines 36-43) as:
import * as whatever from "./whatever.ts";
export function Home(props) {
return jsxDEV_7x81h0kn("p", {
children: props.title
}, undefined, false, undefined, this);
}That output block references jsxDEV_7x81h0kn with nothing defining it — which is precisely the "where does this identifier come from?" confusion that #7499 reports and this PR fixes.
Why it drifts after this PR
This PR flips Config::default().runtime.auto_import_jsx to true in src/runtime/api/JSTranspiler.rs:100. With no explicit autoImportJSX in the options, new Bun.Transpiler({ loader: 'tsx' }).transformSync(code) on JSX now prepends the runtime import. Running the doc's snippet against a build with this change produces:
import { jsxDEV as jsxDEV_7x81h0kn } from "react/jsx-dev-runtime";
import * as whatever from "./whatever.ts";
export function Home(props) {
return jsxDEV_7x81h0kn("p", {
children: props.title
}, undefined, false, undefined, this);
}The documented output no longer matches what Bun emits.
Step-by-step
- User reads
docs/runtime/transpiler.mdxand copies thetransformSyncexample verbatim. Config::default()now setsauto_import_jsx: true(JSTranspiler.rs:100); noautoImportJSXoption is passed, so it stays true.get_parse_result→transpiler.parseruns withauto_import_jsx = true, injecting thereact/jsx-dev-runtimeimport statement.- The printed output has an extra leading
import { jsxDEV as jsxDEV_7x81h0kn } from "react/jsx-dev-runtime";line that the doc's output block doesn't show. - A user diffing their result against the doc sees a mismatch; a user reading only the doc still sees an apparently-undefined
jsxDEV_7x81h0kn— the exact symptom this PR eliminates in the runtime.
Why nothing already covers this
The PR updates packages/bun-types/bun.d.ts to document the new default and updates the three existing tests in transpiler.test.js that asserted on import-free output by adding autoImportJSX: false. But docs/runtime/transpiler.mdx is not touched, and the doc example uses the default (no autoImportJSX key), so it now documents stale behavior. 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 .mdx output block is effectively one such assertion.
Fix
Add the auto-injected import as the first line of the output block in docs/runtime/transpiler.mdx:
```ts output
+import { jsxDEV as jsxDEV_7x81h0kn } from "react/jsx-dev-runtime";
import * as whatever from "./whatever.ts";
export function Home(props) {Severity
Nit — doc drift only, no runtime impact. But it's worth doing in this PR because (a) the doc snippet is the exact call shape whose default this PR changes, and (b) the stale block is the only place in the docs that shows the mangled JSX identifier, so leaving it without the defining import keeps the very confusion #7499 filed alive in the documentation.
1 participant
Fixes #7499.
Problem
Bun.Transpilerwith the automatic JSX runtime (the default) rewrites<div />into a call against a generated symbol likejsxDEV_7x81h0kn("div", ...)whose hash suffix guards against shadowing by user locals. Theimport { jsxDEV as jsxDEV_7x81h0kn } from "react/jsx-dev-runtime"statement that defines that symbol is only emitted whenautoImportJSXis true, and the option defaulted to false.That output references an identifier that nothing defines. Running it fails with
ReferenceError: Can't find variable: jsxDEV_7x81h0kn, and feeding it back intoBun.buildfrom a plugin (the #7499 repro) fails the same way.Fix
Default
autoImportJSXto true inBun.Transpiler, matching the bundler, the module loader, and what the automatic runtime is defined to do. The classic runtime (jsx: "react") has no auto-import and is unaffected.autoImportJSX: falsestill opts out.Verification
New test in
test/bundler/transpiler/transpiler.test.jsfails on main (Expected to start with: "import {") and passes with this change. The three existing JSX-mechanics tests that assert on exact output now passautoImportJSX: falseto keep their expectations focused on the transform body.