Repository navigation
Conversation
…id-js" tsconfig_json.rs special-cased jsxImportSource values starting with "solid-js" to Runtime::Solid, a leftover from the Solid.js JSX transform that was removed in 88538b7. The parser only recognises Automatic vs Classic, so Solid fell through to the classic transform and emitted React.createElement regardless of the configured import source. Drop the override so "solid-js" is treated like any other jsxImportSource (automatic runtime, importing from solid-js/jsx-runtime), matching esbuild and TypeScript. Also fold the vestigial Runtime::Solid to Automatic in to_parser_jsx_pragma so the CLI --jsx-runtime=solid and bunfig jsx = "solid" paths no longer degrade to React.createElement. Fixes #5429
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Duplicate of #35442, which landed the same |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — removing the dead solid-js special-case and folding Runtime::Solid to Automatic is the right fix, and the tests mirror the existing ReactJSXNotReact cases. The inline nit about the stale doc comment is worth addressing but not blocking.
Extended reasoning...
Overview
Three-file change: (1) src/resolver/tsconfig_json.rs deletes the 5-line block that overrode jsx.runtime to Runtime::Solid whenever jsxImportSource started with "solid-js"; (2) src/bundler/transpiler.rs widens the normalization in to_parser_jsx_pragma from _None → Automatic to _None | Solid → Automatic, so the CLI --jsx-runtime=solid and bunfig jsx = "solid" paths (which still produce api::JsxRuntime::Solid) also stop degrading to the classic transform; (3) two new itBundled cases in test/bundler/esbuild/tsconfig.test.ts covering "jsx": "react-jsx" + "jsxImportSource": "solid-js" and "jsx": "preserve" + "jsxImportSource": "solid-js".
Security risks
None. This is bundler config parsing — no untrusted-input handling, no auth/crypto, no memory-safety-relevant native code touched beyond a two-token matches! widening.
Level of scrutiny
Low-to-medium. The removed code was dead weight left over from a transform deleted in 88538b7; the parser only distinguishes Automatic vs Classic (verified in visit_expr.rs per the PR description), so Solid was silently falling into the classic React.createElement path. The removal makes solid-js behave identically to every other jsxImportSource value (matching esbuild/TypeScript). The old behaviour threw ReferenceError: React is not defined at runtime, so nobody could have been depending on it.
I checked the remaining producers of Runtime::Solid (bunfig.rs:960, Arguments.rs:49, jsx.rs:33) — all flow through to_parser_jsx_pragma at both call sites (transpiler.rs:1542, ParseTask.rs:2405), so the fold catches every path to the parser. JSBundler.rs:40 and test/parallel/runner.rs:505 are reverse mappings / display and unaffected.
Other factors
- Tests are placed correctly next to the sibling
ReactJSXNotReact/ReactJSXDevNotReactcases and follow the sameexternal+onAfterBundleshape. - The one finding is a nit: the doc comment above
to_parser_jsx_pragma(and its mirror atParseTask.rs:2402) still says "Only the_None → Automaticfold", which is now inaccurate. Pure documentation drift; the new inline comment at lines 989-991 already explains the Solid case correctly. - Removing
jsx_flags.insert(JsxField::Runtime)for the solid-js case meansmerge_jsxno longer forces a runtime from this tsconfig field alone — but that's consistent with how every otherjsxImportSourcevalue already behaves (the second new test covers exactly thispreserve+ import-source shape).
| // `Solid` is a leftover from the removed Solid.js transform (88538b7c2c); | ||
| // the parser only distinguishes Automatic vs Classic, and any other value | ||
| // would fall through to `React.createElement`. | ||
| if matches!(p.runtime, Runtime::_None | Runtime::Solid) { |
There was a problem hiding this comment.
🟡 nit: the doc comment just above (line 980) still says "Only the _None → Automatic fold is applied", which is no longer accurate now that Runtime::Solid is folded here too. The mirror comment at src/bundler/ParseTask.rs:2402 ("applies the _None → Automatic runtime fold") is stale for the same reason — worth updating both while you're here.
Extended reasoning...
What's stale
The block comment above to_parser_jsx_pragma at src/bundler/transpiler.rs:980 reads:
Only the
_None → Automaticfold is applied so parser-side== Automaticchecks in visitExpr/parseJSXElement keep their pre-unification semantics
This PR changes the function body at line 992 from if p.runtime == Runtime::_None to if matches!(p.runtime, Runtime::_None | Runtime::Solid), so the fold now also maps Runtime::Solid → Automatic. The word "Only" in the doc comment is now factually wrong — two runtime values are folded, not one.
The same description is mirrored at src/bundler/ParseTask.rs:2401-2402:
to_parser_jsx_pragmaapplies the_None → Automaticruntime fold the oldFrombridge did so parser-side== Automaticchecks keep their semantics.
That comment is now equally stale.
Why it matters
Per REVIEW.md, comments must carry durable, accurate content, and every consumer of a fact should be updated atomically ("One source of truth; update every consumer atomically"). The new inline comment at lines 989-991 correctly explains the Solid case, but the pre-existing doc comment nine lines above it — and its mirror in ParseTask.rs — now contradict the code they describe. A future reader skimming the doc comment would conclude _None is the only value normalized here, then be surprised to find Solid handled too.
Step-by-step proof
- Before this PR,
to_parser_jsx_pragmaat line 989 was:if p.runtime == Runtime::_None { p.runtime = Runtime::Automatic; }. The doc comment at line 980 ("Only the_None → Automaticfold is applied") accurately described that single-value fold. - This PR changes line 992 to
if matches!(p.runtime, Runtime::_None | Runtime::Solid) { p.runtime = Runtime::Automatic; }. Now two enum values fold toAutomatic. - The doc comment at line 980 was not touched by this PR. It still says "Only the
_None → Automaticfold", which no longer matches the body directly beneath it. src/bundler/ParseTask.rs:2402describes the same helper the same way ("applies the_None → Automaticruntime fold") and was likewise not updated.
Impact
None behaviorally — this is pure documentation drift. The code is correct; only the surrounding comments misdescribe it.
Fix
Reword line 980 to something like "The _None/Solid → Automatic fold is applied so parser-side == Automatic checks…" (or just drop "Only" and generalize: "Unresolved/vestigial runtime values are folded to Automatic so…"). Apply the same wording to src/bundler/ParseTask.rs:2402.
…#41699) Supersedes #35442 ### Problem - `jsx = "solid"` in bunfig.toml, `--jsx-runtime=solid`, and a tsconfig with `jsxImportSource: "solid-js"` all emit `React.createElement(...)` with no warning. A Solid project then fails at run time with `ReferenceError: React is not defined`. - The Solid.js JSX transform was removed in 88538b7 (Dec 2022). The `Runtime::Solid` enum variant and the three option paths that produce it survived. The parser (`src/js_parser/visit/visit_expr.rs`, the `e_jsx_element` visitor) only implements `Classic` and `Automatic`. It treats every other runtime as `Classic`. ### Fix - Remove `Solid` from `api::JsxRuntime` (`src/options_types/schema.rs`) and `jsx::Runtime` (`src/options_types/jsx.rs`), and every arm that mapped to it. No serialized format carries this enum, so the value change is safe. - bunfig `jsx = "solid"` now fails with the existing "Invalid jsx runtime" error, with `'solid'` removed from the listed values. `--jsx-runtime=solid` now fails with `Invalid --jsx-runtime: "solid", expected "automatic" or "classic"` instead of `An internal error occurred (InvalidJSXRuntime)`. A hard error was chosen over a silent alias to `automatic` because `solid` was never documented for either option (`docs/runtime/bunfig.mdx`, `docs/runtime/jsx.mdx`, the `--jsx-runtime` help text) and a silent alias would hide the same class of mistake this PR removes. - tsconfig `jsxImportSource: "solid-js"` no longer forces a runtime. It behaves like `preact` or any other import source: the automatic runtime with imports from `solid-js/jsx-runtime`. This matches TypeScript and esbuild. It is the same `tsconfig_json.rs` deletion as #35442, which deferred the enum removal to a separate change. - Verified: `test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts` (5 new cases fail on bun 1.4.3 and pass with this change). Also `test/bundler/transpiler/transpiler.test.js`, `test/bundler/esbuild/tsconfig.test.ts`, `test/bundler/bundler_jsx.test.ts`. ### Background - Bun has two JSX transforms. `classic` emits `React.createElement(tag, props, ...children)` and needs a `React` in scope. `automatic` emits `jsx(tag, props)` and imports `jsx` from `<jsxImportSource>/jsx-runtime`. - The old Solid transform compiled JSX to DOM expressions, the way `babel-preset-solid` does. That is a different output shape from both transforms above, and it is what was deleted in 88538b7. - This change does not make Solid work out of the box. `solid-js/jsx-runtime` is a types-only entry point. A Solid project still needs a plugin such as `bun-plugin-solid`. What changes is that Bun no longer emits code that references an undefined `React` global for these configs. <details><summary>Notes</summary> - Repro on 1.4.3: `printf 'jsx = "solid"\n' > bunfig.toml`, `printf 'console.log(<a />);\n' > index.tsx`, `bun build --no-bundle index.tsx` prints `console.log(/* @__PURE__ */ React.createElement("a", null));`. Same with `--jsx-runtime=solid` and no bunfig. - #35460 folded `Solid` to `Automatic` instead of removing it. It was closed as a duplicate of #35442. - #41643 (case-insensitive bunfig `jsx` matching) edits the same lines in `src/bunfig/bunfig.rs` and still lists `solid`. Whichever lands second needs a small rebase. - `Bun.Transpiler` does not auto-inject the jsx import (`autoImportJSX` is off by default there), so the new `Bun.Transpiler` cases assert on the `jsx_...(` / `jsxDEV_...(` call shape instead of the import line. - Self-reviewed: 6 concerns raised, all addressed (issue links, supersede note, #41643 coordination note, `Bun.Transpiler` coverage including `jsx: "preserve"`, plain statement that Solid does not work without a plugin, reason for the hard error). </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 9 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 5 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts bun test v1.4.3 (f42e980) test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts: (pass) tsconfig compilerOptions.jsx > "react-jsx" selects the matching automatic runtime [908.74ms] (pass) tsconfig compilerOptions.jsx > "react-jsxdev" selects the matching automatic runtime [754.71ms] 84 | cwd: String(dir), 85 | stdout: "pipe", 86 | stderr: "pipe", 87 | }); 88 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 89 | expect(stdout).toContain('"solid-js/jsx-runtime"'); ^ error: expect(received).toContain(expected) Expected to contain: "\"solid-js/jsx-runtime\"" Received: "// m.jsx\nvar a = /* @__PURE__ */ React.createElement(\"div\", {\n p: \"1\"\n}, \"x\");\nexport {\n a\n};\n" at <anonymous> (/workspace/bun/test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts:89:20) (fail) tsconfig compilerOptions.jsx > jsxImportSource "solid-js" uses the automatic runtime [ ... (truncated) release without fix: 5 FAILED bun test v1.4.3-canary.1 (f42e980) test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts: (pass) tsconfig compilerOptions.jsx > "react-jsx" selects the matching automatic runtime [24.53ms] (pass) tsconfig compilerOptions.jsx > "react-jsxdev" selects the matching automatic runtime [83.32ms] 84 | cwd: String(dir), 85 | stdout: "pipe", 86 | stderr: "pipe", 87 | }); 88 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 89 | expect(stdout).toContain('"solid-js/jsx-runtime"'); ^ error: expect(received).toContain(expected) Expected to contain: "\"solid-js/jsx-runtime\"" Received: "// m.jsx\nvar a = /* @__PURE__ */ React.createElement(\"div\", {\n p: \"1\"\n}, \"x\");\nexport {\n a\n};\n" at <anonymous> (/workspace/bun/test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts:89:20) (fail) tsconfig compilerOptions.jsx > jsxImportSource "solid-js" uses the automatic runtime [48.58ms] 99 | const transpiler = new Bun.Transpiler({ 100 | loader: "tsx", 101 | tsconfig: { compilerOptions: { jsx, jsxImportSource: "solid-js" } }, 102 | ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts bun test v1.4.3 (f42e980) test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts: (pass) tsconfig compilerOptions.jsx > "react-jsx" selects the matching automatic runtime [1285.82ms] (pass) tsconfig compilerOptions.jsx > "react-jsxdev" selects the matching automatic runtime [776.64ms] (pass) tsconfig compilerOptions.jsx > jsxImportSource "solid-js" uses the automatic runtime [355.94ms] (pass) tsconfig compilerOptions.jsx > Bun.Transpiler with jsx "react-jsx" and jsxImportSource "solid-js" uses the automatic runtime [18.32ms] (pass) tsconfig compilerOptions.jsx > Bun.Transpiler with jsx "preserve" and jsxImportSource "solid-js" uses the automatic runtime [7.49ms] (pass) removed solid jsx runtime > bunfig.toml jsx = "solid" is an error [203.39ms] (pass) removed solid jsx runtime > --jsx-runtime=solid is an error [311.98ms] 7 pass 0 fail 22 expect() calls Ran 7 tests across 1 file. [7.98s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision 9fc97f8 features lto, baseline 24 deps, 131 codegen, 2175 objects in 4819ms ninja: Entering directory `/workspace/bun/build/release' [1/2863] mkdir pch [2/2863] mkdir obj [3/2863] mkdir codegen [4/2863] mkdir stamps [5/2863] gen ErrorCode+*.h [6/2863] fetch mimalloc [mimalloc] up to date [7/2863] fetch icu [icu] up to date [8/2863] fetch WebKit [WebKit] up to date [9/2863] fetch tinycc [tinycc] up to date [10/2863] gen bindgenv2 [11/2863] gen .bind.ts → GeneratedBindings.cpp [12/2863] gen JSBuffer.lut.h Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp [13/2863] gen bake.{client,server,error}.js -> bake.client.js, bake.server.js, bake.error.js [14/2863] gen ProcessBindingConstants.lut.h Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp [15/2863] fetch zlib [zlib] up to date [16/2863] gen CombinedDomains.json [17/2863] cc ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/bunfig/bunfig.rs | 4 +- src/options_types/jsx.rs | 4 +- src/options_types/schema.rs | 1 - src/resolver/tsconfig_json.rs | 5 -- src/runtime/api/JSBundler.rs | 1 - src/runtime/cli/Arguments.rs | 18 +++--- src/runtime/cli/test/parallel/runner.rs | 1 - src/runtime/error.rs | 3 - .../transpiler/jsx-tsconfig-react-jsx.test.ts | 74 ++++++++++++++++++++++ 9 files changed, 86 insertions(+), 25 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/bunfig/bunfig.rs 1 2 6 src/options_types/jsx.rs 1 1 6 src/options_types/schema.rs 1 2 6 src/resolver/tsconfig_json.rs 0 0 6 src/runtime/api/JSBundler.rs 0 0 6 src/runtime/cli/Arguments.rs 1 1 7 src/runtime/cli/test/parallel/runner.rs 0 0 6 src/runtime/error.rs 0 0 6 test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts 1 4 6 ``` </details> <!-- robobun:evidence:end -->
Fixes #5429
Fixes #3528
Problem
With a tsconfig like
{ "compilerOptions": { "jsx": "preserve", "jsxImportSource": "solid-js" } }bun buildandbun runemitReact.createElement(...)instead of using the automatic runtime withsolid-jsas the import source. This breaks every SolidJS project out of the box:The same tsconfig with
"jsxImportSource": "preact"(or any other package) works correctly and imports frompreact/jsx-dev-runtime.Cause
tsconfig_json.rsspecial-cased anyjsxImportSourcestarting with"solid-js"toRuntime::Solid, a value left over from the Solid.js JSX transform that was removed in 88538b7 ("Deprecate very incomplete Solid.js JSX transform"). The parser only recognisesAutomaticvsClassic(visit_expr.rs), soSolidsilently fell through to the classic transform and emittedReact.createElementwith the defaultReactfactory, ignoring the configured import source entirely.Because the override ran after the
"jsx"field was processed, it also clobbered an explicit"jsx": "react-jsx".Fix
solid-jsprefix override intsconfig_json.rsso"jsxImportSource": "solid-js"is treated like any other import source (automatic runtime, importing fromsolid-js/jsx-runtime/solid-js/jsx-dev-runtime). This matches esbuild and TypeScript.Runtime::SolidtoAutomaticinto_parser_jsx_pragma, so--jsx-runtime=solidand bunfigjsx = "solid"no longer degrade toReact.createElementeither.After:
solid-jsships bothjsx-runtimeandjsx-dev-runtimeentry points, so this runs. It is not the optimised Solid compilation (that needsbabel-preset-solidvia a plugin, e.g.bun-plugin-solid), but it is the correct generic behaviour forjsxImportSourceand no longer references an undefinedReactglobal.Verification
New
tsconfig/ReactJSXSolidandtsconfig/PreserveImportSourceSolidcases intest/bundler/esbuild/tsconfig.test.tscover both tsconfig shapes from the issue. They fail on currentbun(emitReact.createElement) and pass with this change.bundler_jsx.test.ts,esbuild/tsconfig.test.ts,transpiler.test.js, andjsx-tsconfig-react-jsx.test.tspass unchanged.