inspect: print an own enumerable Symbol.toStringTag and hide the non-enumerable __esModule marker on the fast property walk too

[robobun]

Problem

• console.log({ a: 1, [Symbol.toStringTag]: "Tag" }) prints Tag { a: 1 }, but adding any getter to the same object makes the key appear: Tag { a: 1, g: [Getter], [Symbol(Symbol.toStringTag)]: "Tag" }. Bun.inspect(obj, { sorted: true }) and node print the key in both cases.
• console.log(require("./compiled.cjs")) for a tsc/babel-compiled module prints the non-enumerable __esModule: true marker when the module only has data exports, and hides it as soon as the module has a re-export getter. Node hides it in both cases.
• Cause: JSC__JSValue__forEachPropertyImpl in src/jsc/bindings/bindings.cpp has two enumeration paths that apply different rules to these two keys. The fast path (structure->forEachProperty lambda, line 5528 on main) skipped Symbol.toStringTag unconditionally and __esModule whenever it came from a prototype; the slow path (line 5635) and forEachPropertyOrdered (line 5799) skip them only when the property is non-enumerable. Which path an object takes is an implementation detail (a getter or indexed properties, for example, force the slow path), so the output depended on it.

Fix

• The fast path now hides __proto__, Symbol.toStringTag and __esModule under the same condition as the slow path: only when the entry is DontEnum. No other line changes; the constructor skip and the slow path are untouched.
• Why non-enumerable is the right line: the non-enumerable form is how classes and builtins brand themselves (get [Symbol.toStringTag]() on a class prototype, Object.defineProperty(prototype, Symbol.toStringTag, ...)) and how compiled CommonJS marks itself (Object.defineProperty(exports, "__esModule", { value: true })). That is the form the skip exists for, and it stays hidden. An enumerable one was put there by a literal or an assignment and is user data; the formatter already prints every other enumerable property, including inherited ones, and two of the three walkers already printed these.
• The __esModule rule that is dropped (hide when inherited) was added in Fixes #14411 #14691 alongside the slow path's DontEnum rule, for the __esModule accessor bun installs on the prototype of module namespace objects. That accessor is DontEnum, so it is still hidden (and namespace objects never take the fast path anyway).
• Output changes only for objects printed through the fast path that own or inherit an enumerable Symbol.toStringTag (now printed) or own a non-enumerable __esModule (now hidden), which in both cases is what the same object already printed once it had a getter. { [Symbol.toStringTag]: "x" } with no other properties already printed the key (the empty fast walk fell through to the slow path), so the committed console-log.expected.txt is unchanged.
• Not changed: forEachPropertyOrdered (expect diffs, snapshots, sorted: true) never hides __esModule; it is a separate walker with a consistent rule of its own, and changing it would alter existing snapshots.
• Verified with bun bd test test/js/bun/util/inspect.test.js: the new Symbol.toStringTag property and __esModule property tests cover the fast, slow (getter) and sorted walkers for own enumerable, own non-enumerable and prototype properties; 4 of the 5 fail on the build without this change, the fifth (non-enumerable stays hidden) guards against over-correcting. test/js/web/console, test/js/bun/console, test/cli/run/esm-defineProperty.test.ts, test/js/bun/resolve/esModule*.test.* and the rest of test/js/bun/util pass.

Background

• console.log and Bun.inspect do not enumerate properties themselves; the Rust formatter calls JSC__JSValue__forEachProperty, which lists the object's own properties and then walks up to five prototypes so that class methods are shown. When the object's JSC Structure allows it (no accessors, no indexed properties, no custom property lookup) it reads the property table directly (fast path); otherwise it calls getOwnPropertyNames and looks each property up (slow path). forEachPropertyOrdered is a third walker that lists own properties sorted by name; it backs sorted: true and the test runner's diff printer.
• DontEnum is JSC's attribute for enumerable: false. Class methods and accessors, Object.defineProperty defaults and bun's native prototypes use it; object literals and plain assignment create enumerable properties. Both formatters print non-enumerable properties in general (that is how prototype methods show up), which is why hiding the branding keys needs an explicit rule rather than falling out of an enumerable-only listing.

Before / after for the shapes the tests cover

                                                    before (fast path)        after
{ a: 1, [Symbol.toStringTag]: "Tag" }               Tag { a: 1 }              Tag { a: 1, [Symbol(Symbol.toStringTag)]: "Tag" }
same object with a getter (slow path)               prints the key            unchanged
defineProperty(obj, Symbol.toStringTag, ...)        Tag { a: 1 }              unchanged
class with get [Symbol.toStringTag]()               K { a: 1 }                unchanged
defineProperty(K.prototype, Symbol.toStringTag)     K { a: 1, method }        unchanged
K.prototype[Symbol.toStringTag] = "K" (enumerable)  K { a: 1, method }        K { a: 1, method, [Symbol(Symbol.toStringTag)]: "K" }
  (same class with a getter already printed the key)

defineProperty(exports, "__esModule", {value:true}) { __esModule: true, foo } { foo }
  (same exports object with a re-export getter already printed { foo, bar: [Getter] })
{ __esModule: true, foo: 1 } (enumerable)           prints the key            unchanged
Object.create({ __esModule: true, method(){} })     { x, method }             { x, __esModule: true, method }
  (same prototype with a getter already printed __esModule)

[robobun]

^{Updated 10:05 AM PT - Aug 15th, 2026}

❌ @robobun, your commit dfce16b has 1 failures in Build #98084 (All Failures):

• test/cli/install/bun-install-registry.test.ts - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 39045

That installs a local version of the PR into your bun-39045 executable, so you can run:

bun-39045 --bun

[robobun]

Status: reproduced and fixed, waiting on CI.

Reproduced on bun 1.4.0 with Bun.inspect({ a: 1, [Symbol.toStringTag]: "Tag" }) (key hidden) vs the same object with a getter (key printed), and with a tsc-style exports object whose non-enumerable __esModule printed only when the module had no re-export getters. The new Symbol.toStringTag property / __esModule property tests in test/js/bun/util/inspect.test.js fail on the build without the bindings.cpp change and pass with it.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 72cc2e55-09e4-42cf-8b36-fa25de65cd57

📥 Commits

Reviewing files that changed from the base of the PR and between 88a6398 and dfce16b.

📒 Files selected for processing (2)

• src/jsc/bindings/bindings.cpp
• test/js/bun/util/inspect.test.js

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — aligns the fast property walk's skip rule for __proto__/Symbol.toStringTag/__esModule with the slow path's existing DontEnum gate.

What was reviewed:

• Confirmed the new fast-path condition at bindings.cpp:5534 exactly matches the slow-path rule at :5642 and the ordered walker's rule at :5806 (which never skipped __esModule, as the description notes).
• Checked that constructor is still unconditionally skipped and that the dropped objectToUse != object guard for __esModule is subsumed by the DontEnum check for the module-namespace prototype accessor it targeted.
• Tests cover fast/slow/sorted × own/prototype × enumerable/non-enumerable for both keys, including the guard case that non-enumerable stays hidden.

Extended reasoning...

Overview

This PR changes one if condition in JSC__JSValue__forEachPropertyImpl's fast-path lambda in src/jsc/bindings/bindings.cpp. Previously the fast path unconditionally hid __proto__ and Symbol.toStringTag, and hid __esModule only when inherited; the slow path (getOwnPropertyNames-based) already hid all three only when DontEnum. The change makes the fast path use the same DontEnum gate, so Bun.inspect output no longer depends on which enumeration path an object happens to take. Five new test blocks in test/js/bun/util/inspect.test.js cover the matrix.

Security risks

None. This is purely presentational — it affects which property keys Bun.inspect/console.log prints for an object. No parsing of untrusted input, no auth/crypto/permissions, no memory management changes. entry.attributes() is already read on the line above the change.

Level of scrutiny

Low-to-moderate. The C++ change is a single condition rewrite; I verified against the current source that the slow path (bindings.cpp:5642-5646) and forEachPropertyOrdered (bindings.cpp:5806-5810) already implement the exact rule the fast path is being aligned to, so this is a consistency fix rather than a new policy. The constructor skip is preserved unchanged. The dropped objectToUse != object heuristic for __esModule is explained in the description (added for the module-namespace prototype's __esModule accessor, which is DontEnum and so still hidden).

Other factors

• Test coverage is thorough: each of the three walkers (fast, slow via getter, sorted) is exercised for own-enumerable, own-non-enumerable, and prototype cases of both Symbol.toStringTag and __esModule. One test is an explicit no-regression guard (non-enumerable stays hidden).
• The PR description enumerates the before/after for every shape and cross-references the related console/esModule test suites that were run.
• No prior reviewer comments to address; CI is building. The change is small, self-contained, and the reasoning for each dropped/kept clause is documented.

[github-actions]

This PR may be a duplicate of:

1. fix(console): console.log respects enumerable:false #38218 - Applies a blanket PropertyAttribute::DontEnum skip to the same fast structure walk in JSC__JSValue__forEachPropertyImpl, subsuming the "hide the non-enumerable __esModule marker" half of this change, and rewrites the slow-path underscoreProto || toStringTagSymbol || __esModule exemption this PR is matching.
2. Print own enumerable "constructor" properties in expect diffs, snapshots and console.log #38937 - Rewrites the exact same if (prop == constructor || underscoreProto || toStringTagSymbol || ...) statement with the identical "skip only when non-enumerable" rule, applied to constructor instead of Symbol.toStringTag/__esModule.

🤖 Generated with Claude Code

[robobun]

Not a duplicate of either, though both touch the same statement:

• Print own enumerable "constructor" properties in expect diffs, snapshots and console.log #38937 applies the same "skip only when non-enumerable" rule to constructor and deliberately leaves Symbol.toStringTag alone (its description calls it out as a separate pre-existing inconsistency). The two are complementary; whichever lands second has a one-line textual conflict in the fast-path if.
• fix(console): console.log respects enumerable:false #38218 adds a blanket DontEnum skip to all three walkers, which is a much broader output change (it also drops the prototype methods console.log prints today). It keeps the fast path's unconditional Symbol.toStringTag skip, so with it an own enumerable Symbol.toStringTag is still hidden on the fast path and printed on the slow path, which is the bug this PR fixes. Only the __esModule half here would become redundant if fix(console): console.log respects enumerable:false #38218 landed.