Repository navigation
Conversation
… Bun.build jsx options Follow-up to #34422, which made tsconfig "react-jsx" emit the production automatic runtime instead of the development one. The behavior now matches TypeScript and esbuild, but users upgrading from a version that always emitted jsxDEV may not expect the change. - docs/runtime/jsx.mdx: add a Note under the jsx table spelling out that "react-jsx" = prod / "react-jsxdev" = dev, point users who want DevTools source locations at "react-jsxdev", and record the pre-1.4 behavior. - packages/bun-types/bun.d.ts: document each value of TSConfig compilerOptions.jsx, and add per-field JSDoc to the Bun.build jsx config object (runtime, importSource, factory, fragment, sideEffects, development).
|
Docs only ( The CI red on build 85808 is |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
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 (1)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…t tree-shaking); split runtime from-clause
There was a problem hiding this comment.
LGTM — all three earlier notes are addressed (sideEffects now describes the per-call pure annotation, the runtime from-clause is split, and the dev-runtime entries no longer claim a source location is emitted).
What was reviewed:
- Re-checked each JSDoc claim against
src/options_types/jsx.rsandsrc/resolver/tsconfig_json.rs—"preserve"falling back to the automatic runtime and the@defaultvalues match the implementation. - Confirmed the
<Note>wording is consistent with the transpiled-output table above it and withvisit_expr.rs(undefinedsourcearg). - Docs +
.d.tsonly; nosrc/change, so no runtime risk.
Extended reasoning...
Overview
Docs/types-only follow-up to #34422: adds a <Note> to docs/runtime/jsx.mdx explaining that "react-jsx" now selects the production automatic runtime (with the pre-1.4 behavior called out), and adds JSDoc to TSConfig.compilerOptions.jsx and every field of BuildConfig.jsx in packages/bun-types/bun.d.ts. No src/ files touched.
Prior feedback
My earlier review flagged three inaccuracies: sideEffects described import tree-shaking rather than the per-call pure annotation; the runtime doc implied jsxDEV came from /jsx-runtime; and two entries claimed Bun passes source location to jsxDEV. Commits 97258c7 and f15636f address all three, and the current diff reflects the corrected wording. I re-verified each against src/js_parser/visit/visit_expr.rs (can_be_unwrapped_if_unused gated on jsx.side_effects; hard-coded undefined for the source arg) and src/options_types/jsx.rs (IMPORT_SOURCE_DEV = "react/jsx-dev-runtime").
Security risks
None. Pure documentation and JSDoc comments in .d.ts; no executable code, no auth/crypto/permission surface.
Level of scrutiny
Low. Docs and type-declaration comments only — the CLAUDE.md exception for packages/bun-types/**/*.d.ts applies (no build required). I spot-checked the remaining factual claims: "preserve" falling through to Automatic (RUNTIME_MAP has no preserve entry, Pragma::default().runtime == Automatic), and the @default values ("react", "React.createElement", "React.Fragment", "automatic", false) all match src/options_types/jsx.rs.
Other factors
The bug-hunting pass on the updated revision found nothing. The bun-types integration test passes per the PR description. The <Note> uses the same MDX component pattern as neighboring notes on the page.
React fires missing-key warnings from react/jsx-runtime too (gated on NODE_ENV, not on jsxDEV), so the example was misleading. The "derived from NODE_ENV and tsconfig" default clause for BuildConfig.jsx.development does not match current Bun.build behavior (tsconfig is not consulted and an explicit value can be overridden by NODE_ENV); removed rather than documenting in-flux precedence.
JSDoc comments in .d.ts have no runtime or type-check observable effect, so there is no fail-before test the gate can run for them. The docs/runtime/jsx.mdx Note is the primary visibility deliverable.
robobun
changed the title
There was a problem hiding this comment.
LGTM — docs-only <Note> addition to docs/runtime/jsx.mdx.
What was reviewed
- Verified the note's claims against
src/options_types/jsx.rs:51-53:"react-jsx"→development: Some(false)(production),"react-jsxdev"→Some(true). - Multi-line
<Note>…</Note>syntax matches existing usage across the docs tree. - Earlier inline comments were on
packages/bun-types/bun.d.ts, which was dropped in 61584a5; nothing from those applies to the remaining diff.
Extended reasoning...
Overview
Single-file docs change: adds a <Note> block under the jsx compiler-options table in docs/runtime/jsx.mdx clarifying that tsconfig "react-jsx" selects the production automatic runtime and "react-jsxdev" selects the development one, and recording that pre-1.4 Bun treated "react-jsx" as the dev runtime. The packages/bun-types/bun.d.ts JSDoc that earlier review rounds commented on was removed in 61584a5, so the final diff is 9 added lines of MDX and nothing else.
Security risks
None. No executable code, no build config, no type surface — pure prose in a .mdx file.
Level of scrutiny
Low. Docs-only, no src/ or packages/ change, and per .claude/docs/landing-prs.md docs guidance the bar is factual accuracy. I checked the two factual claims that touch implementation:
"react-jsx"= production /"react-jsxdev"= development: confirmed inRUNTIME_MAPatsrc/options_types/jsx.rs:51-53, where"react-jsx"maps todevelopment: Some(false)and"react-jsxdev"toSome(true). This also matches the pre-existing table rows immediately above the new note.- "matching TypeScript and esbuild": consistent with tsc's documented
react-jsx/react-jsxdevsplit and esbuild'sjsx: "automatic"+jsxDevbehavior.
The "Prior to Bun 1.4" version reference tracks #34422; I could not independently confirm the exact release number from the local checkout (no tags / gh unavailable), but it is a low-risk historical note and the PR description shows the author verified current behavior against a debug build.
Other factors
- The
<Note>component with a blank line before</Note>is used the same way in ~66 other doc files, so MDX rendering should be fine. - All three prior inline threads from my earlier review are marked resolved and targeted the now-removed
.d.tshunk; none apply to the remainingjsx.mdxchange. - The note deliberately avoids documenting
NODE_ENV/--jsx-*override precedence while #36269 is in flight, which keeps this change self-contained. - CI red on the referenced build is an unrelated worker stress test, not this docs file.
#34422 made tsconfig
"jsx": "react-jsx"select the production automatic runtime (jsx/jsxsfrom<source>/jsx-runtime) instead of the development one (jsxDEVfrom<source>/jsx-dev-runtime), matching TypeScript and esbuild. The JSX docs page already showedreact-jsx->jsx-runtimein its table, so the table itself is correct, but nothing on the page said that this is what distinguishes the two values or that it changed.Why
The output for the most common React tsconfig value changed. Projects upgrading from a Bun that always emitted
jsxDEVwill see the import target switch fromjsx-dev-runtimetojsx-runtime, and the reason is not discoverable from the docs. A short note makes it discoverable without changing any runtime behavior.Change
docs/runtime/jsx.mdx: add a<Note>under thejsxtable that spells out"react-jsx"= production /"react-jsxdev"= development (matching tsc/esbuild), points at"react-jsxdev"for thejsxDEVentry point, and records the pre-1.4 behavior.A build-time warning was considered and rejected: tsc, esbuild and Babel do not warn,
"react-jsx"is the valuetsc --initwrites, and the new output is what every other tool already produces.Verification
Docs only; no
src/orpackages/change. The claims were verified against a debug build at c08f665:Related
NODE_ENV/--jsx-*precedence inbun build; this PR deliberately does not document the override precedence while that is in flight.TSConfig.compilerOptions.jsxandBuildConfig.jsxwas drafted and reviewed on this branch (bf5c795) but dropped: JSDoc comments have no fail-before-observable effect. Happy to restore in a follow-up if wanted.no test proof · iteration 1 · docs-only change; test-proof not applicable