js_parser: don't fold wrappers that expose anonymous class/fn to NamedEvaluation

[robobun]

What

bun run (no flags) gives anonymous classes/functions a .name the spec leaves empty:

const A = (0, class {});
const B = true && class {};
const C = [class {}][0];
const K = ({ prop: class {} }).prop;
const fn = (0, function () {});
function param(P = (0, class {})) { return P.name; }
class Holder { field = (0, class {}); }
console.log(JSON.stringify({ A: A.name, B: B.name, C: C.name, K: K.name, fn: fn.name, param: param(), field: new Holder().field.name }));
// node:  {"A":"","B":"","C":"","K":"prop","fn":"","param":"","field":""}
// bun:   {"A":"A","B":"B","C":"C","K":"K","fn":"fn","param":"P","field":"field"}

Why

The runtime transpiler forces minify_syntax = true for target.is_bun(), which enables constant folding at these sites:

• BinComma: (0, expr) -> expr
• BinLogicalAnd / BinLogicalOr / BinNullishCoalescing: true && expr -> expr
• EIf: true ? expr : _ -> expr
• EIndex on EArray: [expr][0] -> expr
• maybe_rewrite_property_access on EObject: ({k: expr}).k -> expr

When expr is an anonymous class/function/arrow, printing the folded result into a binding position (const A = class {}, default parameter, class field, object property) triggers NamedEvaluation and the binding name sticks. The (0, expr) idiom exists precisely to defeat this, so folding it away breaks real code (DI/registry keyed on constructor.name).

The object-literal case is worse: the spec assigns the property key ("prop") as the name, so folding replaces a correct name with the wrong one.

Fix

Guard each fold site with is_anonymous_named(), mirroring the existing is_call_target && has_value_for_this_in_call() pattern that already protects this binding. For comma/logical/ternary/array, emit (0, expr) instead of a bare anonymous class/function/arrow. For the object-literal property access, skip the fold (the property key name must be preserved).

Named classes/functions still fold normally since is_anonymous_named() returns false for them.

Tests

Added to test/bundler/transpiler/runtime-transpiler.test.ts: all wrapper forms across const, default parameter, and class field positions, plus a guard test that direct const A = class {} still gets name "A" and (0, class Named {}) still folds.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 14 minutes

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: 16af32ce-808c-4aba-b220-cf106f3eda1f

📥 Commits

Reviewing files that changed from the base of the PR and between df6c7ee and aadf954.

📒 Files selected for processing (4)

• src/js_parser/fold.rs
• src/js_parser/visit/visit_binary.rs
• src/js_parser/visit/visit_expr.rs
• test/bundler/transpiler/runtime-transpiler.test.ts

---

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

[robobun]

^{Updated 6:02 AM PT - Jul 25th, 2026}

@robobun, your commit aadf954 is building: #80805

[claude]

Both of my earlier findings are addressed in ce4639e — the return e_.left paths in BinLogicalOr/BinNullishCoalescing are now wrapped, the EIf CouldHaveSideEffects branch seeds 0 via needs_comma_wrap before falling back to E::Missing, and the new H/I/J/L cases cover them. I didn't find further gaps this pass, but this touches ~10 fold sites in the parser hot path and there are outstanding comment-cop flags, so leaving for a human sign-off.

What was reviewed:

• Re-checked all guarded fold sites (comma, &&/||/?? both operands, ternary both arms × both side-effect paths, [x][0], {k:x}.k, {}.k ??= x) against is_anonymous_named().
• Verified needs_comma_wrap hoist also covers the pre-existing is_call_target case on the CouldHaveSideEffects path.
• Checked the ({}).x ??= class {} test expectation against spec (MemberExpression LHS → no NamedEvaluation → "" is correct).

Extended reasoning...

Overview

The PR guards constant-folding sites in src/js_parser/{fold.rs, visit/visit_binary.rs, visit/visit_expr.rs} so that folding a wrapper expression ((0, x), true && x, [x][0], cond ? x : y, ({k:x}).k, ({}).k ??= x) down to a bare anonymous class/function/arrow does not expose it to NamedEvaluation at the enclosing binding site. It mirrors the existing is_call_target && has_value_for_this_in_call() pattern with is_anonymous_named(), emitting (0, expr) instead of a bare expr. Three runtime tests were added to test/bundler/transpiler/runtime-transpiler.test.ts.

My previous review (on commit 3758bcc) flagged two unguarded sibling paths: the return e_.left arms of BinLogicalOr/BinNullishCoalescing, and the CouldHaveSideEffects branch of the EIf fold where simplify_unused_expr → None → E::Missing → join_with_comma returned the arm bare. Commit ce4639e addresses both — the left-operand arms now wrap via join_with_comma(ZERO, e_.left), and the ternary CouldHaveSideEffects path chains .or_else(|| needs_comma_wrap.then(...)) ahead of the E::Missing fallback. The test file gained H ([] ? class {} : 0), I ((function(){}) || 0), J ((()=>{}) ?? 0), and L ((class{}) ?? 0) covering exactly those cases.

Security risks

None. This is AST-level constant folding; inputs are already-parsed expression nodes and the change only decides whether to keep a (0, …) comma wrapper.

Level of scrutiny

High. visit_binary.rs and visit_expr.rs run on every file the runtime transpiler touches, and the fix pattern is repeated across ~10 sibling sites — the class of bug this PR fixes is exactly "missed one parallel arm", so it deserves careful human eyes to confirm the enumeration is now complete (I did not find further unguarded fold-to-bare sites, but this is a whack-a-mole shape). The needs_comma_wrap hoist in the EIf handler also slightly changes behavior for the is_call_target case on the CouldHaveSideEffects path (now wrapped where it previously wasn't) — that reads as a correct incidental fix, but is worth a maintainer glance.

Other factors

There are four outstanding comment-cop bot flags on the added inline comments. The flagged comments are short (3-4 lines, same style as the neighboring // "(0, this.fn)()" => "(0, this.fn)()" examples) and read as legitimate why-comments rather than workaround justifications, but they are unaddressed automated feedback and the author should decide whether to trim them. Given the outstanding bot flags plus the breadth of parser sites touched, deferring rather than approving.

[claude]

Both prior findings are addressed — the left-operand return e_.left sites in BinLogicalOr/BinNullishCoalescing and the ternary CouldHaveSideEffects path are now guarded, with matching test cases (I/J/L/H). No new issues found. Deferring only because this is parser constant-folding that runs on every file; worth a maintainer glance at the approach (guard-per-fold-site vs. handling at the binding position).

What was reviewed:

• Traced is_anonymous_named() (arrow / unnamed fn / unnamed class only) and prefill::data::ZERO → ENumber(0.0), so join_with_comma never short-circuits to bare expr.
• Checked ({}).x ??= class {} against the spec — LHS is not an IdentifierRef, so no NamedEvaluation; test expectation X: "" is correct.
• The guards are strictly additive (mirror the existing is_call_target && has_value_for_this_in_call() shape); non-anonymous operands are unaffected.

Extended reasoning...

Overview

Guards ~8 constant-folding sites in the JS parser (BinComma, BinLogicalAnd/Or, BinNullishCoalescing, BinNullishCoalescingAssign|BinLogicalOrAssign, EIf both arms × both side-effect branches, EIndex on EArray × 2, EObject property access) so that folding a wrapper down to a bare anonymous class/function/arrow doesn't expose it to NamedEvaluation at the enclosing binding. Emits (0, expr) instead, or skips the fold for the object-literal case where the property key is the spec-assigned name. ~80 lines Rust + 3 test.concurrent blocks.

Security risks

None. Pure AST-transform correctness; no I/O, no untrusted input handling changes.

Level of scrutiny

High — this is the runtime transpiler's constant-folding path, executed on every file bun run loads. That said, every change is a strictly conservative guard added to an existing fold: when the operand is not an anonymous fn/class/arrow, behavior is byte-identical. The (0, expr) wrap is the same mechanism the pre-existing is_call_target && has_value_for_this_in_call() guard already uses at each of these sites, so there's a known-good template right next to every hunk.

Other factors

• My two earlier findings (left-operand ||/?? folds; ternary CouldHaveSideEffects + simplify_unused_expr → None → E::Missing path) were fixed in ce4639e and covered by new test inputs I, J, L, H.
• Verified prefill::data::ZERO is ExprData::ENumber, so join_with_comma cannot short-circuit via is_missing() on the new sites.
• Verified the ({}).x ??= class {} test expectation against the spec: logical-assignment NamedEvaluation only fires when IsIdentifierRef of the LHS is true; a MemberExpression LHS skips it, so .name === "" is correct and the added !e_.right.is_anonymous_named() guard preserves that.
• Tests cover all three anonymous forms, both operands, and include a negative test confirming direct const A = class {} still gets "A" and named classes still fold.
• Outstanding comment-cop bot lints appear addressed by 99bed96 (all added comments are one-line // "in" => "out" examples matching neighboring style), but they haven't auto-resolved.

Not approving solo because parser folding is load-bearing for every file and a maintainer should sign off on scattering the guard across 8 sites vs. centralizing it.

[robobun]

Status: ready for review.

Pass-after verified locally (host disk recovered): bun bd test test/bundler/transpiler/runtime-transpiler.test.ts 18/18 pass, transpiler.test.js 182/182 pass, simplifier-side-effects.test.ts 1/1 pass. Fail-before confirmed with USE_SYSTEM_BUN=1 (all 15 wrapper forms get wrong .name).

CI: builds #80517 and #80805 were both fleet-wide infra outages (169 waiting_failed, 20-25 expired, build-cpp sibling errored with nothing to link). My Rust code compiles clean on the CI darwin-aarch64 lane that did run. Re-roll is spent; needs a maintainer to re-trigger or merge once CI recovers.

On the design question raised in review: the guard is unconditional (not keyed on minify_keep_names) because this is spec correctness, not name preservation. (0, class {}).name === "" is required behavior regardless of minifier settings; esbuild does the same.

[claude]

The prior sibling-site findings look addressed in ce4639e and the guards now cover left-operand ||/?? and the ternary CouldHaveSideEffects path. Deferring to a human on this one — it's ~10 fold-site edits in the parser hot path, the author couldn't run bun bd test locally (relying on CI #80805), and it's worth a maintainer confirming the guards should be unconditional rather than gated on minify_keep_names (they also affect bun build --minify output size, not just the runtime transpiler).

Checked: is_anonymous_named() covers EArrow/unnamed EFunction/unnamed EClass; join_with_comma won't short-circuit the new (0, ...) wraps since prefill::data::ZERO isn't EMissing; the ({}).x ??= class {} test expectation of "" matches spec (NamedEvaluation for ??= requires IsIdentifierRef on the LHS, which a MemberExpression fails); BinLogicalAnd's unguarded return e_.left is unreachable for anonymous fn/class since they're always truthy.

Extended reasoning...

Overview

Guards ~10 constant-folding sites in src/js_parser/{fold.rs, visit/visit_binary.rs, visit/visit_expr.rs} so that folding a wrapper expression ((0, x), true && x, null ?? x, cond ? x : y, [x][0], ({k:x}).k, ({}).k ??= x) down to a bare anonymous class/function/arrow emits (0, x) instead — preventing NamedEvaluation from assigning the binding name where the spec says .name should be "" (or the property key, in the object-literal case). Adds three test.concurrent subprocess tests covering 15 wrapper forms plus a guard test that direct const A = class {} and named classes still fold normally.

Security risks

None. Pure AST-transform correctness change; no I/O, no untrusted input parsing beyond what already exists.

Level of scrutiny

High. This is the JS parser's constant-folding path, exercised on every file the runtime transpiler touches (minify_syntax is force-on for target.is_bun()). A wrong guard here could regress correctness or output size across all of bun run and bun build --minify. The change is mechanically consistent (mirrors the pre-existing is_call_target && has_value_for_this_in_call() pattern at each site), but the number of touched sites and the earlier round of missed siblings argue for a human pass.

Other factors

• Prior review feedback addressed: my earlier inline findings (unguarded left-operand ||/?? returns, ternary CouldHaveSideEffects arms) are now covered — verified against to_boolean/to_null_or_undefined in scan_side_effects.rs and join_with_comma's is_missing() short-circuit.
• Local verification gap: the author reports bun bd link failing locally and is relying on CI (#80805, still building at last update). REVIEW.md requires pass-after confirmation; a maintainer should confirm CI is green before merge.
• Design question: the guards are unconditional, so bun build --minify will now emit (0, class {}) where it previously emitted class {}. That's a strict spec-correctness improvement and matches the existing this-binding pattern, but a maintainer may want to weigh whether it should instead key off minify_keep_names (which exists in parser.rs) — esbuild is the reference here per REVIEW.md.
• Test quality: tests follow harness conventions (test.concurrent, bunEnv/bunExe, drain stdout/stderr/exited concurrently, assert stderr before exit code), and the guard test proves the fold isn't over-broad. Fail-before was confirmed with USE_SYSTEM_BUN=1.

[robobun]

The comma case was independently re-reported, so confirming this is still live on current main (checked with a build at b7a0431, which already includes #36730):

const o = { a: (0, function () {}), b: (0, class {}), c: (0, () => {}) };
const d = (0, function () {});
let e; e = (0, function () {});
console.log(JSON.stringify([o.a.name, o.b.name, o.c.name, d.name, e.name]));
// node: ["","","","",""]
// bun:  ["a","b","c","d","e"]

Every wrapper form in the test added here (&&, ||, ??, ternary, [x][0], ({k: x}).k, default parameter, class field) still produces the same wrong names on that build, so the full set of guards is still needed.

The branch now conflicts with main in src/js_parser/fold.rs, src/js_parser/visit/visit_binary.rs and src/js_parser/visit/visit_expr.rs (#36730 reworked the [x][0] / {f: x}.f folds and #36766 changed the SideEffects result type), so it needs a rebase before CI can run.

[robobun]

The [x][0] case was independently re-reported as well, still live on main at 2f5c180 (which includes #36730):

const B = [class {}][0];
const f = [function () {}][0];
const g = [0, class {}][1];
console.log(JSON.stringify([B.name, f.name, g.name]));
// node: ["","",""]
// bun:  ["B","f","g"]

bun build --no-bundle --minify-syntax prints const B = class {}, f = function() {}, g = class {}. This idiom is how a number of tests (including node's own suite) obtain an anonymous class, so it also shows up as [class B] instead of [class (anonymous)] in util.inspect output and in error messages. After the rebase, #36730 merged the single-item and multi-item array paths into one inlined site in visit_expr.rs, so a single is_anonymous_named() guard there covers both shapes above. No separate PR is being opened for this; it is covered here.

[robobun]

The object-literal case was independently re-reported too. Still live on main at 2f5c180 (src/js_parser/fold.rs has no name guard on the EObject fold; checked against a build at b7a0431, which includes #36730):

const a = ({ aa: async () => {} }).aa;
function f() { return ({ bb: () => {} }).bb; }
const c = ({ "c-c": function () {} })["c-c"];
const d = ({ dd: class {} }).dd;
console.log(JSON.stringify([a.name, f().name, c.name, d.name]));
// node v26.3.0: ["aa","bb","c-c","dd"]
// bun:          ["a","","c","d"]

This PR's !value.is_anonymous_named() guard in fold.rs covers it (and the "({ prop: ... }).prop" test here already exercises class, function and arrow values), so I am not opening a separate PR. Two notes for the rebase:

• js_parser: stop the [x][0] / {f:x}.f folds from changing optional-chain / this / assignment semantics #36730 landed a value.can_be_inlined_from_property_access() check on the line this PR's fold.rs hunk is anchored to, which is what makes the branch conflict now.
• js_parser: fold property access on multi-property object literals #36599 (open) extends the same fold to multi-property object literals and returns the matching value without going through that single-property condition, so if it lands too, its path needs the same guard.

[robobun]

The [x][0] case came up again with an arrow function, still reproducible on main at d3f975b:

const B = [() => null][0];
let C = [function () {}][0];
var D = [class {}][0];
console.log(JSON.stringify([B.name, C.name, D.name]));
// node: ["","",""]
// bun:  ["B","C","D"]

This branch covers it (is_anonymous_named() is true for arrows), so no separate fix is being opened for it. Note that the branch currently conflicts with main: #36730 restructured the array fold in src/js_parser/visit/visit_expr.rs into a single inlined path with an is_call_target && has_value_for_this_in_call() guard, so the name guard now belongs next to that check.

[robobun]

The object literal site of this PR is now split out as #42585.

#40833 (merged to main on 2026-09-13) made the standard decorator lowering emit h = (effects, { h: () => {} }["h"]) to keep the name of a field that carries the effects of its neighbors. The { k: v }.k fold removes that wrapper when bun runs lowered output again (bun build --no-bundle, bun build --target=node, Bun.Transpiler), so new A().h.name becomes "". That is a regression on main, and this branch cannot run CI until it is rebased, so #42585 carries only the !value.is_anonymous_named() condition in src/js_parser/fold.rs, with tests for the decorator round trip.

When this branch is rebased, drop its fold.rs hunk and the ({ prop: class {} }).prop test if #42585 has merged. The other fold sites here (comma, && / || / ??, ternary, [x][0]) are not touched by #42585.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-25, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.

[robobun]

Continued in #42691, against current main.

It carries the comma, &&, ||, ??, ternary, [x][0] and ({}).x ??= guards of this PR in the shape that #36730 and #36766 left, behind one helper. A call target stays unwrapped, so the [() => y][0]() expectation from #36730 holds. The { f: fn }.f guard stays in #42585.