bun:test: format React 19 elements as JSX in snapshots and matcher diffs - #38934
bun:test: format React 19 elements as JSX in snapshots and matcher diffs#38934robobun wants to merge 4 commits into
Conversation
The test runner's value formatter only recognized an object as a React
element when its $$typeof was Symbol.for("react.element") or
Symbol.for("react.fragment"). React 19 marks elements with
Symbol.for("react.transitional.element"), so toMatchSnapshot,
toMatchInlineSnapshot and the Expected/Received output of toEqual,
toStrictEqual, toMatchObject and the mock call matchers dumped those
elements as plain objects, while Bun.inspect already printed them as JSX.
Check the same set of symbols as the console formatter. Child elements go
through the same check, so nested React 19 trees print as JSX too.
|
Warning Review limit reached
Next review available in: 1 minute 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 |
|
Reproduced on bun 1.4.0 with a hand-built element ( Fix in this PR: CI: every test job that ran is green (177 of 179 on the latest build). The remaining two are the |
There was a problem hiding this comment.
LGTM — brings the test-runner formatter in line with ConsoleObject.rs for React 19's Symbol.for("react.transitional.element").
What was reviewed:
Tag::getloop: same three symbol keysConsoleObject.rschecks; the refactor from two chainedis_same_valuecalls to a loop over aconstarray preserves behavior forreact.element/react.fragment.- Snapshot tests:
describe.eachruns every case (props, key, component type, single/array children, nested in object/array) for both symbols, so React 18 output is proven unchanged. - Diff test: uses
tempDir/bunEnv, drains stdout/stderr/exited concurrently, and the regex extractor is anchored so an empty match set would fail the inline snapshot rather than pass vacuously.
Extended reasoning...
Overview
The PR adds b"react.transitional.element" to the set of $$typeof symbol keys that Tag::get in src/runtime/test_runner/pretty_format.rs classifies as Tag::JSX, and rewrites the two chained is_same_value checks as a loop over a three-entry const [&[u8]; 3]. Two test files gain coverage: snapshot.test.ts adds a describe.each over both symbol keys exercising toMatchInlineSnapshot for elements with props, keys, component types, child elements, array children, and elements nested inside objects/arrays; test-test.test.ts adds an it.each that spawns a fixture whose toEqual/toStrictEqual/not.toEqual fail and snapshots the matcher diff output.
Security risks
None. This is a display-only classification change in the test-runner value formatter. No parsing of untrusted input, no allocation-size arithmetic, no auth/crypto/permissions surface.
Level of scrutiny
Low. The functional change is a single new byte-string entry in a list whose other two entries were already present, aligning this formatter with src/jsc/ConsoleObject.rs:2253-2257 which already checks the same three keys. The loop refactor uses the exact same helpers (ZigString::init, JSValue::symbol_for, is_same_value) with ? propagation preserved, and short-circuits on the first match just as the || chain did. No new error paths, no lifetime or GC concerns (ZigString::init borrows a 'static slice per iteration).
Other factors
The tests follow repo conventions closely: tempDir + using, bunEnv/bunExe, Promise.all over stdout/stderr/exited, inline snapshots with exact JSX output, and expect(exitCode) asserted after content. Running both symbol keys through the same assertions guards against regressing React 18 formatting. The PR description notes both files were run under bun bd test and that the react.transitional.element cases fail on the unfixed build, satisfying the fails-for-the-right-reason bar. The one noted pre-existing local failure (error snapshots when stderr is not a TTY) is unrelated and tracked separately.
…utput shape Nest the multi-line elements in an object so the snapshot only depends on how the elements are classified, and re-indent the diff inline snapshot the way bun test writes it.
There was a problem hiding this comment.
LGTM — the comment nit from the earlier pass is addressed in fcb787e.
What was reviewed:
Tag::getinpretty_format.rs: the loop overREACT_ELEMENT_SYMBOLSpreserves the existingreact.element/react.fragmentpaths and?-propagates fromis_same_valueexactly as before; only the newreact.transitional.elemententry changes behavior.- Confirmed the symbol set matches the sibling check in
ConsoleObject.rs:2250-2257. - Test coverage:
.eachover both symbol keys, so thereact.elementcases guard against regression whilereact.transitional.elementcases exercise the fix; snapshot, child-element, array, and matcher-diff paths are all covered.
Extended reasoning...
Overview
The PR adds Symbol.for("react.transitional.element") (React 19's element marker) to the set of $$typeof symbols that Tag::get in src/runtime/test_runner/pretty_format.rs recognizes as a JSX element, so React 19 elements serialize as JSX in bun test snapshots and matcher diffs instead of as plain object dumps. The Rust change is ~15 lines: two chained is_same_value checks become a loop over a 3-entry const array, with the new symbol as the middle entry. Two test files add describe.each/it.each blocks over both react.element and react.transitional.element covering inline snapshots (single prop, key, component type, child element, array children, element-in-array) and matcher diffs (toEqual, toStrictEqual with children, not.toEqual).
Security risks
None. This is display-only formatting for test-runner output; no untrusted input reaches new allocation, syscall, or FFI paths. The refactored loop calls the same ZigString::init / JSValue::symbol_for / is_same_value helpers as before, with ? on the fallible call preserved.
Level of scrutiny
Low. The change mirrors the already-landed fix in ConsoleObject.rs (#17223) at the sibling formatter — I verified the symbol list at ConsoleObject.rs:2252-2257 matches. A React 19 element differs from a React 18 element only in this symbol, so routing it down the existing Tag::JSX path is the whole fix; the JSX printer itself is unchanged. The refactor from ||-chain to loop is behavior-preserving for the two existing symbols (same early-return on first match, same exception propagation).
Other factors
My one prior comment on this PR (multi-line code comment) and the comment-cop flag were both addressed in fcb787e — the header is back to one line and the react.transitional.element entry carries the #17223 link, matching the ConsoleObject.rs convention. All inline threads are resolved. The tests follow harness conventions (tempDir, bunEnv, concurrent pipe drain, toMatchInlineSnapshot on normalized output, exit code asserted last), and the .each structure means the react.element half guards the pre-existing behavior against regression. The bug-hunting pass found nothing.
Problem
bun testprints a React 19 element as a plain object in snapshots (toMatchSnapshot,toMatchInlineSnapshot) and in the Expected/Received output oftoEqual,toStrictEqual,toMatchObject,toHaveBeenCalledWithand the other matchers that render a diff:<div id="x" />, andBun.inspect/console.logprint the React 19 one as JSX too.bun testdisagrees withconsole.logabout the same value.Tag::getinsrc/runtime/test_runner/pretty_format.rs(line 495) only compares$$typeofagainstSymbol.for("react.element")andSymbol.for("react.fragment"). React 19 marks elements withSymbol.for("react.transitional.element"). The console formatter (src/jsc/ConsoleObject.rs, line 2249) was taught that symbol in the fix for JSX logging no longer works in react 19 #17223; the test runner formatter was not.Tag::get, so a React 19 element whoseprops.childrenis an element (or an array of them) also prints its children as objects.Fix
Tag::getchecks the same three symbols as the console formatter. The fixing change is thereact.transitional.elemententry inREACT_ELEMENT_SYMBOLS; the rest of the hunk turns the two chained comparisons into a loop over that list. Nothing else changes: a React 19 element now takes exactly the path a React 18 element takes.bun testprints for one is what it should print for the other;ConsoleObject.rsalready treats the symbol that way, and so does vitest'spretty-format(itsReactElementplugin accepts an element if either the React 18 or the React 19 copy ofreact-isdoes).Bun.markdown.react()also emitsreact.transitional.elementelements by default..snapthat holds a React 19 element was written in the object form and needsbun test -uafter this change. React 18 snapshots are unchanged (every test below runs with both symbols)..eachblocks below can take a multi-prop element.toMatchObjectwith a partial pattern against an element diffs the JSX form against the pattern object, as it does for React 18 today. That is atoMatchObjectdiff issue (jest diffs the received subset) and is tracked separately.test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts,inline snapshots of %s elements: hand-built elements (test/ pins React 18, and the symbol is the only thing that differs) with a prop, a key, a component type, a single child element, array children, and an element inside an array. Thereact.transitional.elementcases fail on the unfixed build with the object dump above; thereact.elementcases pass before and after.test/js/bun/test/test-test.test.ts,expect() diffs print %s elements as JSX: spawnsbun teston a file whosetoEqual,toStrictEqual(child elements) andnot.toEqualassertions fail, and snapshots the matcher output. Same fail-before pattern.bun bd test. The one other failure insnapshot.test.tslocally (error snapshots) also fails on main when stderr is not a TTY and is addressed by test: make the error snapshots test pass with colors disabled #38833.Background
{ $$typeof, type, key, ref, props }.$$typeofis a registered symbol that marks the object as an element. React 18 and older useSymbol.for("react.element"); React 19 renamed it toSymbol.for("react.transitional.element")so that elements created by the two versions are not mistaken for each other.src/jsc/ConsoleObject.rsbacksconsole.logandBun.inspect;src/runtime/test_runner/pretty_format.rsis the jestpretty-formatequivalent used to serialize snapshots and to render the Expected/Received values thatDiffFormatterdiffs. Each one classifies a value with its ownTag::getbefore printing, and theTag::JSXresult selects that formatter's JSX printer.