Skip to content

Fix crash when inspecting circular or malformed JSX elements #30383

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.

Sign up for GitHub

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

Closed
robobun wants to merge 1 commit into from
+43 −8
Closed

Fix crash when inspecting circular or malformed JSX elements #30383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/jsc/ConsoleObject.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1136,7 +1136,7 @@ pub const Formatter = struct {

pub fn canHaveCircularReferences(tag: Tag) bool {
return switch (tag) {
.Function, .Array, .Object, .Map, .Set, .Error, .Class, .Event => true,
.Function, .Array, .Object, .Map, .Set, .Error, .Class, .Event, .JSX => true,
else => false,
};
}
Expand Down Expand Up @@ -3140,13 +3140,12 @@ pub const Formatter = struct {
}
}

if (try value.get(this.globalThis, "props")) |props| {
if (if (try value.get(this.globalThis, "props")) |props| props.getObject() else null) |props_obj| {
const props = props_obj.toJS();
const prev_quote_strings = this.quote_strings;
defer this.quote_strings = prev_quote_strings;
this.quote_strings = true;

// SAFETY: JSX props are always objects
const props_obj = props.getObject().?;
var props_iter = try jsc.JSPropertyIterator(.{
.skip_empty_name = true,
.include_value = true,
Expand Down
7 changes: 3 additions & 4 deletions src/test_runner/pretty_format.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ pub const JestPrettyFormat = struct {
}

pub inline fn canHaveCircularReferences(tag: Tag) bool {
return tag == .Array or tag == .Object or tag == .Map or tag == .Set;
return tag == .Array or tag == .Object or tag == .Map or tag == .Set or tag == .JSX;
}

const Result = struct {
Expand Down Expand Up @@ -1534,13 +1534,12 @@ pub const JestPrettyFormat = struct {
}
}

if (try value.get(this.globalThis, "props")) |props| {
if (if (try value.get(this.globalThis, "props")) |props| props.getObject() else null) |props_obj| {
const props = props_obj.toJS();
Comment on lines +1537 to +1538

@coderabbitai coderabbitai Bot May 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Compare React element symbol coverage between the two formatter implementations.
rg -n -C2 'react\.(element|transitional\.element|fragment)' \
  src/test_runner/pretty_format.zig \
  src/jsc/ConsoleObject.zig

Repository: oven-sh/bun

Length of output: 1629

React 19 transitional elements still bypass the JSX formatter.

This file detects only "react.element" and "react.fragment" (lines 419-422) but not "react.transitional.element", unlike src/jsc/ConsoleObject.zig (line 1307). React 19 elements will skip the hardened JSX path added in this PR and fall back to generic object formatting.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test_runner/pretty_format.zig` around lines 1537 - 1538, The JSX
formatter currently only recognizes "react.element" and "react.fragment" and
therefore skips React 19 transitional elements; update the tag-name check that
matches "react.element" and "react.fragment" to also accept
"react.transitional.element" (the same place that branches into the hardened JSX
path) so that when you extract props (the block using value.get(this.globalThis,
"props") -> props_obj -> props_obj.toJS()) transitional elements follow the JSX
formatting path just like the other React element types.

const prev_quote_strings = this.quote_strings;
defer this.quote_strings = prev_quote_strings;
this.quote_strings = true;

// SAFETY: JSX props are always an object.
const props_obj = props.getObject().?;
var props_iter = try jsc.JSPropertyIterator(.{
.skip_empty_name = true,
.include_value = true,
Expand Down
37 changes: 37 additions & 0 deletions test/js/bun/util/inspect.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,3 +772,40 @@ it("CustomEvent", () => {
}"
`);
});

describe("JSX element", () => {
it("handles circular key", () => {
const elem = {
$$typeof: Symbol.for("react.element"),
type: "div",
key: null,
ref: null,
props: { children: [] },
};
elem.key = elem;
expect(Bun.inspect(elem)).toContain("[Circular]");
});

it("handles circular children", () => {
const elem = {
$$typeof: Symbol.for("react.element"),
type: "div",
key: null,
ref: null,
props: {},
};
elem.props.children = elem;
expect(Bun.inspect(elem)).toContain("[Circular]");
});

it("handles non-object props", () => {
const elem = {
$$typeof: Symbol.for("react.element"),
type: "div",
key: null,
ref: null,
props: 123,
};
expect(Bun.inspect(elem)).toBe("<div />");
});
Comment on lines +777 to +810

@coderabbitai coderabbitai Bot May 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n test/js/bun/util/inspect.test.js | sed -n '770,820p'

Repository: oven-sh/bun

Length of output: 1236

🏁 Script executed:

rg "describe\.each" test/js/bun/util/inspect.test.js -A 5 -B 2

Repository: oven-sh/bun

Length of output: 37

🏁 Script executed:

rg "describe\.each" test/js/bun/ --type js --type ts -l | head -20

Repository: oven-sh/bun

Length of output: 627

🏁 Script executed:

cat test/js/bun/test/jest-each.test.ts | head -100

Repository: oven-sh/bun

Length of output: 1609

🏁 Script executed:

rg "describe\.each|it\.each" test/js/bun/test/jest-each.test.ts -A 8 -B 1 | head -80

Repository: oven-sh/bun

Length of output: 1455

🏁 Script executed:

rg "it\.each" test/js/bun/test/jest-each.test.ts -A 5 -B 1 | head -40

Repository: oven-sh/bun

Length of output: 1126

🏁 Script executed:

# Check if there are examples of describe.each with complex objects in the test suite
rg "describe\.each.*\(" test/js/bun/ -A 10 -B 1 --type ts --type js | head -60

Repository: oven-sh/bun

Length of output: 4859

🏁 Script executed:

# Let's verify the exact syntax expected by checking how describe.each callback should work
cat test/js/bun/udp/udp_socket.test.ts | sed -n '1,50p'

Repository: oven-sh/bun

Length of output: 2056

🏁 Script executed:

# Check the jest-each test more carefully to understand the callback structure
rg "describe\.each.*\{" test/js/bun/typescript/type-export.test.ts -A 8 -B 2 | head -50

Repository: oven-sh/bun

Length of output: 1323

🏁 Script executed:

# Let's confirm the exact callback signature expected for describe.each
rg "describe\.each.*\)" test/js/bun/udp/udp_socket.test.ts -A 5 -B 1

Repository: oven-sh/bun

Length of output: 358

🏁 Script executed:

# Check if there's any example that directly executes logic in describe.each callback
rg "describe\.each" test/js/bun/ --type ts --type js -A 15 | grep -A 15 "describe\.each.*hostname" | head -25

Repository: oven-sh/bun

Length of output: 1780

🏁 Script executed:

# Let me verify the exact structure needed - check if describe.each can work with verify functions or if we need it.each
cat test/js/bun/dns/resolve-dns.test.ts | grep -A 20 "describe.each(validHostnames)"

Repository: oven-sh/bun

Length of output: 546

Use it.each() for these parameterized test cases.

Lines 777–810 contain repeated test structure with only fixture/assertion differences. Using it.each() will consolidate these tests and align with repo conventions per the coding guidelines.

♻️ Suggested refactor 
 describe("JSX element", () => {
-  it("handles circular key", () => {
-    const elem = {
-      $$typeof: Symbol.for("react.element"),
-      type: "div",
-      key: null,
-      ref: null,
-      props: { children: [] },
-    };
-    elem.key = elem;
-    expect(Bun.inspect(elem)).toContain("[Circular]");
-  });
-
-  it("handles circular children", () => {
-    const elem = {
-      $$typeof: Symbol.for("react.element"),
-      type: "div",
-      key: null,
-      ref: null,
-      props: {},
-    };
-    elem.props.children = elem;
-    expect(Bun.inspect(elem)).toContain("[Circular]");
-  });
-
-  it("handles non-object props", () => {
-    const elem = {
-      $$typeof: Symbol.for("react.element"),
-      type: "div",
-      key: null,
-      ref: null,
-      props: 123,
-    };
-    expect(Bun.inspect(elem)).toBe("<div />");
-  });
+  it.each([
+    {
+      name: "handles circular key",
+      setup: () => {
+        const elem = {
+          $$typeof: Symbol.for("react.element"),
+          type: "div",
+          key: null,
+          ref: null,
+          props: { children: [] },
+        };
+        elem.key = elem;
+        return elem;
+      },
+      assertion: (output) => expect(output).toContain("[Circular]"),
+    },
+    {
+      name: "handles circular children",
+      setup: () => {
+        const elem = {
+          $$typeof: Symbol.for("react.element"),
+          type: "div",
+          key: null,
+          ref: null,
+          props: {},
+        };
+        elem.props.children = elem;
+        return elem;
+      },
+      assertion: (output) => expect(output).toContain("[Circular]"),
+    },
+    {
+      name: "handles non-object props",
+      setup: () => ({
+        $$typeof: Symbol.for("react.element"),
+        type: "div",
+        key: null,
+        ref: null,
+        props: 123,
+      }),
+      assertion: (output) => expect(output).toBe("<div />"),
+    },
+  ])("$name", ({ setup, assertion }) => {
+    assertion(Bun.inspect(setup()));
+  });
 });
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/js/bun/util/inspect.test.js` around lines 777 - 810, Replace the three
separate it(...) cases ("handles circular key", "handles circular children",
"handles non-object props") with a single parameterized it.each(...) table that
supplies a description, a function or factory to build the elem fixture (so you
can create circular references inside the factory for the first two cases), and
the expected assertion type/value; call Bun.inspect on the produced elem in the
test body and branch the assertion: expect(...).toContain("[Circular]") for the
first two fixtures and expect(...).toBe("<div />") for the non-object props
fixture. Ensure you reference the same symbols used now (elem, props, $$typeof,
Bun.inspect) so the factory builds identical shapes and the descriptions match
the original test names.

});
Loading