-
Notifications
You must be signed in to change notification settings - Fork 5k
Bun.Transpiler: default autoImportJSX to true for the automatic JSX runtime #35582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,6 +97,7 @@ | |
| log: bun_ast::Log::default(), // overwritten at construction | ||
| runtime: Runtime::Features { | ||
| top_level_await: true, | ||
| auto_import_jsx: true, | ||
|
Check warning on line 100 in src/runtime/api/JSTranspiler.rs
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The Extended reasoning...What's stale
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 Why it drifts after this PRThis PR flips 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
Why nothing already covers thisThe PR updates FixAdd the auto-injected import as the first line of the output block in ```ts output
+import { jsxDEV as jsxDEV_7x81h0kn } from "react/jsx-dev-runtime";
import * as whatever from "./whatever.ts";
export function Home(props) {SeverityNit — 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. |
||
| ..Default::default() | ||
| }, | ||
| tree_shaking: false, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2086,6 +2086,7 @@ export default <>hi</> | |
| "process.env.NODE_ENV": JSON.stringify("development"), | ||
| }, | ||
| logLevel: "error", | ||
| autoImportJSX: false, | ||
| }); | ||
|
|
||
| expect(bun.transformSync("console.log(<div key={() => {}} points={() => {}}></div>);")).toBe( | ||
|
|
@@ -2154,6 +2155,40 @@ console.log(<div {...obj} key="after" />);`), | |
| ); | ||
| }); | ||
|
|
||
| // 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. | ||
|
Comment on lines
+2158
to
+2161
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 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 AgentsSource: Coding guidelines |
||
| it("JSX automatic runtime import is emitted by default", () => { | ||
| for (const loader of ["jsx", "tsx"]) { | ||
| const out = new Bun.Transpiler({ loader }).transformSync("export default <><div /></>"); | ||
| expect(out).toStartWith("import {"); | ||
| expect(out).toContain('from "react/jsx-dev-runtime"'); | ||
| // Every generated JSX symbol that is called/used must be bound by the import. | ||
| for (const [name] of out.matchAll(/\b(jsxDEV|jsx|jsxs|Fragment|createElement)_\w+\b/g)) { | ||
| expect(out).toContain(` as ${name}`); | ||
| } | ||
| } | ||
|
|
||
| // 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 "); | ||
|
Comment on lines
+2173
to
+2189
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Cover both The default-import assertion iterates over both loaders, but the 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| }); | ||
|
|
||
| // Non-bundle transpile without `minify.identifiers` uses NoOpRenamer | ||
| // (prints symbol.original_name verbatim), so the `generatedSymbolName` | ||
| // hash suffix on the automatic JSX runtime import is the sole collision | ||
|
|
@@ -2203,6 +2238,7 @@ console.log(<div {...obj} key="after" />);`), | |
| loader: "jsx", | ||
| define: { "process.env.NODE_ENV": JSON.stringify("development") }, | ||
| logLevel: "error", | ||
| autoImportJSX: false, | ||
| }); | ||
| process.stdout.write(t.transformSync('console.log(<div key key="duplicate"></div>);')); | ||
| process.stdout.write(t.transformSync('console.log(<div key className="x" key="duplicate"></div>);')); | ||
|
|
@@ -2344,6 +2380,7 @@ console.log(<div {...obj} key="after" />);`), | |
| define: { | ||
| "process.env.NODE_ENV": JSON.stringify("development"), | ||
| }, | ||
| autoImportJSX: false, | ||
| }); | ||
| expect(bun.transformSync("export var foo = <div>{...a}b</div>")).toBe( | ||
| `export var foo = jsxDEV_7x81h0kn("div", { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the development runtime import path.
Automatic development transforms import
<jsxImportSource>/jsx-dev-runtime—for example, the test expectsreact/jsx-dev-runtimeon Line 2166—while this documentation only names/jsx-runtimedespite mentioningjsxDEV.Proposed documentation fix
📝 Committable suggestion
🤖 Prompt for AI Agents