inspect, bun:test: self-close JSX elements whose children are "" or []

RO
12d ago
+103-7

Problem

  • Bun.inspect, console.log and bun:test snapshots print an unterminated tag for some React elements: <ul, or <ul id="x". Those elements have props.children equal to [] or "".
  • Both JSX printers return after the 'print_children block (src/jsc/ConsoleObject.rs:5308, src/runtime/test_runner/pretty_format.rs:2268). The empty-string and empty-array arms break out of it, so the /> write never runs.

Fix

  • Move the return into the block. Both breaks now reach the /> write: <ul />, <ul id="x" />.
  • Correct because printed children still write </tag> and return from the same place.
  • Verified: 10 new cases in test/js/bun/util/inspect.test.js and 9 in test/js/bun/test/snapshot-tests/snapshots/moremore.test.ts fail on bun 1.4.3 and pass here. Also ran md-react.test.ts and snapshot-tests/.
  • Self-reviewed: 15 concerns raised, 14 addressed. Rejected: a review request, since nobody has reviewed this PR.

Background

  • <ul>{[]}</ul> is a plain object whose props.children is []. Bun prints it as JSX.
  • print_jsx serves Bun.inspect and console.log. pretty_format.rs is its copy for snapshots and expect messages. #46729 deletes the copy and, like #44833, keeps this defect.
  • Considered the ConsoleObject.rs hunk alone: stored snapshots stay valid, but bun:test still prints <ul.

Downsides

  • A snapshot that stores <ul fails once. bun test -u rewrites it. #46729 keeps stored snapshot text by default, so this needs a maintainer's yes.
  • A test.each title that prints such an element changes, and so do the snapshot keys of that test.
  • Not fixed: a sole number child is dropped. <b>{5}</b> prints <b />.
Notes

Before and after. Before is bun 1.4.3. After is a debug build of this branch on main 11c367163a. Each row is a React 18 element object, { $$typeof: Symbol.for("react.element"), type, key: null, ref: null, props }.

ValueBeforeAfter
ul, props: { children: [] }<ul<ul />
ul, props: { children: "" }<ul<ul />
ul, props: { id: "x", children: [] }<ul id="x"<ul id="x" />
an array of two such div elements<div, <div<div />, <div />
a fragment with children: []<< />
Bun.markdown.react("")<< />
Bun.markdown.react("# \n\ntext")<h1 in the fragment<h1 /> in the fragment

toMatchSnapshot, toMatchInlineSnapshot and the Expected and Received lines of expect print the same text as the first three rows.

Other printers. For the same objects, @vitest/pretty-format 4.1.9 prints <ul /> for [] and for "". jest pretty-format 29.7.0 prints <ul /> for []. For "" it prints <ul>, a blank line, and </ul>.

Stored snapshots. bun 1.4.3 writes <ul and <ul id="x" into a .snap file and accepts <ul as an inline snapshot. With this change both fail with Expected: <ul and Received: <ul />. bun test -u rewrites the .snap file and the inline snapshot. A scan of 4,819 public Bun .snap files (47,336 entries, found through GitHub code search) found no stored truncated tag.

test.each titles. test.each([[element, 1]])("title %p %d", ...) has the title title <p className="label" 1 on bun 1.4.3 and title <p className="label" /> 1 with this change. A snapshot key contains the title.

Not changed. Both binaries print the same text for these:

  • A sole child that is a number, a bigint, true, null, an object or a function is not printed: <b />. The printers print children only for a string, an element or an array.
  • Array entries print as they are. [""] prints an empty line, [[]] prints [], [null, false] prints null and false.
  • A fragment with no children prints < />.
  • compact: true prints <b /> for any children.

Other PRs.

  • #46729 deletes pretty_format.rs and prints snapshots with print_jsx. At its head (69b78c5e1a) print_jsx still has the return after the block. If #46729 merges first, the pretty_format.rs hunk here goes away and the ConsoleObject.rs hunk fixes both outputs.
  • #44833 keeps the return after the block in both printers (head e5453e1ca9).
  • #22897 fixed the same defect in both printers with a did_print_children flag, together with a larger attribute change. It was closed without a merge. The moved return needs no flag.
  • The defect is in the first version of the JSX printer (69eeff1f76, 2022-03-10). 4792abdb7f copied it into the snapshot printer.

Test placement. The new block in inspect.test.js is below the ErrorEvent test, next to the other jsx tests at the end of the file. ErrorEvent prints a source preview of its own lines, and its snapshot fails when the test is on line 1000 or later. With the block above it, the test is past that line. #47123 removes that dependency.

Local runs. Debug build, main 11c367163a plus this branch:

  • test/js/bun/util/inspect.test.js: 116 pass, 0 fail
  • test/js/bun/test/snapshot-tests/snapshots/moremore.test.ts: 11 pass, 0 fail
  • test/js/bun/md/md-react.test.ts: 73 pass, 0 fail
  • test/js/bun/test/snapshot-tests/ with FORCE_COLOR=1: 90 pass, 1 skip, 1 todo, 0 fail

Self-review. Two more suggestions are not done:

  • The breaking label. #23390 and #46729 change snapshot text without it, and the cost is in Downsides.
  • A tracking issue for the sole child that is not printed.

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/util/inspect.test.js