js_parser: stop the [x][0] / {f:x}.f folds from changing optional-chain / this / assignment semantics - #36730
Conversation
The minify_syntax fold that turns `[x][0]` into `x` was willing to
inline an optional chain expression. When the surrounding index is
itself the start of an optional chain (so the next access is a
continuation), the printer splices the two chains together:
[[a?.b]][0]?.[0].c
became `a?.b.c`, which short-circuits to `undefined` for `a == null`
instead of throwing on the trailing `.c` like the original (and
Node/esbuild) do.
The direct `[a?.b]?.[0].c` form was hidden because the parse-time
`?.`-on-literal simplification drops the outer chain first, but one
level of indirection (`[[a?.b]][0]?.[0].c` or
`({f:[a?.b]}).f?.[0].c`) exposes it on current main.
Teach can_be_inlined_from_property_access to reject EDot/EIndex/ECall
nodes that carry an optional chain, and turn the downstream
debug_assert into a real guard so the multi-item path can bail in
release builds too.
|
Status: diff is green; CI red is unrelated. Reproduced on main:
Three new tests under "property access inlining" in Remaining red on #87482 is unrelated to this diff:
The |
WalkthroughChangesArray-property inlining
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — the optional-chain bailout is correct and narrowly scoped, and the inline note about the pre-existing is_call_target/is_delete_target/assign_target gap at the same fold is worth a follow-up but doesn't block this fix (behavior there is byte-identical before and after).
Extended reasoning...
Overview
Adds three arms to Expr::can_be_inlined_from_property_access so that EDot/EIndex/ECall with a non-None optional_chain are rejected, and converts the multi-item path's debug_assert! on the same predicate into a real runtime guard. The predicate has exactly two callers (visit_expr.rs:1091 and :1108), both at the [x][0] -> x fold, so the blast radius is fully contained. Tests cover all three chain node kinds, the object-literal fold path, non-chain items still inlining, and an end-to-end subprocess check that the TypeError is preserved.
Security risks
None. Pure JS AST simplification predicate; no I/O, allocation, or FFI changes.
Level of scrutiny
Medium — parser/minifier correctness is user-visible via bun run, but the change is a strict narrowing of an existing predicate (previously _ => true covered these variants). It can only cause the fold to bail more often, never fire in new cases, so the failure mode is "slightly less minified" rather than new miscompiles. The debug_assert! → if conversion also removes a release/debug divergence.
Other factors
- Verified the predicate has no other callers in
src/. - Tests follow harness conventions (bunExe/bunEnv, concurrent pipe drain, exit code asserted last) and include negative cases confirming plain
a.band[[y]][0]?.[0].cstill fold. - The inline finding about
[obj.m][0]()/delete [obj.p][0]/[obj.p][0] = 5is pre-existing — those inputs hit the_ => truewildcard on main and produce identical output with or without this patch. It's the same fold site but a different fix shape (needs parent-context checks +(0, x)emission, matching the sibling comma/??/ternary folds), so scoping this PR to the optional-chain splice and taking that as a follow-up is reasonable.
Expand the same fold site to also respect the parent expression: - As a call target: `[obj.m][0]()` now emits `(0, obj.m)()` instead of `obj.m()`, matching the sibling comma/??/||/&&/ternary folds, so `this` inside `m` is not rebound to `obj`. - As an assignment target: `[obj.p][0] = v`, `[,][0] = v`, `"s"[n] = v` stay as written instead of folding to `obj.p = v` / `void 0 = v` / `"s" = v` (the last two are SyntaxErrors in the emitted output). - Add coverage for the Continuation (a?.b.c) and multi-item @__PURE__ removable paths that reach the second fold arm. The `!is_delete_target` check is included for completeness but is currently always false because `p.delete_target` is never set by the UnDelete handler; that is tracked separately.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
robobun
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/js_parser/visit/visit_expr.rs`:
- Around line 1060-1064: Track whether the original EIndex is the delete target
before Op::UnDelete visits its operand, and use that state when applying the
minify_syntax temporary-expression folding guard in the expression visitor. Add
an isolated plain-property regression case for delete [obj.p][0] in
transpiler.test.js, preserving the existing optional-chain coverage.
In `@test/bundler/transpiler/transpiler.test.js`:
- Around line 226-232: Remove the exact-empty stderr assertion from the
subprocess test using Bun.spawn. Continue draining proc.stderr, and validate the
expected outcome through stdout and exitCode (and signal if applicable), leaving
stderr available only for diagnostics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6adaeb19-013b-4156-82c3-3a1f7b0fc2c7
📒 Files selected for processing (3)
src/ast/expr.rssrc/js_parser/visit/visit_expr.rstest/bundler/transpiler/transpiler.test.js
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟣
src/ast/expr.rs:65-67— 🟣 Pre-existing, but the direct sibling of the fold this PR hardens: the{f: x}.f -> xobject-literal fold atsrc/js_parser/fold.rs:487returnsSome(value)without callingcan_be_inlined_from_property_access(), sonew ({f: a?.b}).f()minifies tonew a?.band({f: a?.b}).f`x`minifies toa?.b`x`— both hard SyntaxErrors. The one-line fix is to gate that return onvalue.can_be_inlined_from_property_access(), reusing exactly the predicate this PR just hardened; per REVIEW.md "Fix the whole class in the same PR — grep for every sibling site sharing the pattern", worth folding in here (or as an immediate follow-up).Extended reasoning...
What the bug is
The
{f: x}.f -> xobject-literal fold inmaybe_rewrite_property_access(src/js_parser/fold.rs:459-490) returnsSome(value)at :487 unconditionally once the property matches. Its guards at :470-473 are!is_delete_target && assign_target == None && !is_call_target— there is no check on whethervalueis an optional-chain expression. Whenvalueisa?.band the parent isnewor a tagged template, the minifier emits syntactically invalid JavaScript:$ echo 'x = new ({f: a?.b}).f()' | bun build --minify-syntax --no-bundle /dev/stdin x = new a?.b; // SyntaxError: Invalid optional chain from new expression $ echo 'x = ({f: a?.b}).f`x`' | bun build --minify-syntax --no-bundle /dev/stdin x = a?.b`x`; // SyntaxError: Invalid tagged template on optional chainAlso
new ({f: a?.[b]}).f()→new a?.[b]andnew ({f: a?.b.c}).f()→new a?.b.c(Continuation on the outermost node).Step-by-step proof (
newcase)Input:
new ({f: a?.b}).f()e_newvisits its target without settingp.call_target(visit_expr.rs:2323-2326 does a plainp.visit_expr(&mut e_.target); onlye_callat :1854 setsp.call_target). So whene_dotruns on.f, it computesis_call_target = falseat :1349.e_dotcallsmaybe_rewrite_property_accessat :1434, gated one_.optional_chain.is_none()— true for the plain.f. It passesis_call_target=false, is_delete_target=false, assign_target=None.- The object fold fires (fold.rs:470-487): single property, all three guards pass,
INLINE_PROPERTIES_IN_TRANSPILER = true, key matches"f". ReturnsSome(a?.b)— anEDotwithoptional_chain = Some(Start). - The
ENewprinter does not wrap it. js_printer/lib.rs:3048 prints the target atLevel::NewwithExprFlag::forbid_call()only — it does not passHasNonOptionalChainParent. TheEDotprinter arm (:3256-3306) has no level-based paren wrap; it wraps only whenflags.contains(HasNonOptionalChainParent)(:3271), which is false here. Output:new a?.b— a SyntaxError in every JS engine.
Step-by-step proof (tagged-template case)
Input:
({f: a?.b}).f`x`e_template(visit_expr.rs:696) visits its tag via a plainp.visit_expr— again nocall_targettracking, so the object fold fires exactly as above and the tag becomes the rawa?.b.- The
ETemplateprinter's optional-chain wrap check at js_printer/lib.rs:3751 matches onexpr.data(theETemplatenode itself), not ontag.data, so it always falls through to_ => false. The tag then prints at :3762 withExprFlag::none()— noHasNonOptionalChainParent. Output:a?.b`x`— SyntaxError.
(The
ECallvariantnew ({f: a?.()}).f()happens to survive because theECallprinter wraps atlevel.gte(Level::New), butEDot/EIndexdo not.)Why this PR's fix does not cover it
can_be_inlined_from_property_access()— the predicate this PR hardens to reject optional-chainEDot/EIndex/ECall— has exactly one caller: the array fold at visit_expr.rs:1113. The object fold at fold.rs:487 never calls it.The chain-splice variant this PR primarily targets cannot reach the object fold, because both callers of
maybe_rewrite_property_access(visit_expr.rs:1037 fore_index, :1435 fore_dot) are gated one_.optional_chain.is_none()— so the parent can never be aContinuation. But thenewand tagged-template parent positions still miscompile, because neithere_newnore_templatesets any tracking the fold could consult, and neither printer passesHasNonOptionalChainParentto its target.Why existing guards don't catch it
The object fold's only guards are
!is_delete_target && assign_target == None && !is_call_target. There is no optional-chain check onvalue, andis_call_targetisfalseundernew/template-tag anyway.Relationship to this PR
This PR fixes the identical bug for the array fold — its own new tests at transpiler.test.js cover
x = new [a?.b][0]()and[a?.b][0]`x`(both now bail via the hardened predicate). The object fold is the direct sibling: same fold shape (single-item literal → item), same problematic parent contexts, same predicate would fix it. REVIEW.md: "Fix the whole class in the same PR — grep for every sibling site sharing the pattern."Fix
One line at fold.rs:485 — add
&& value.can_be_inlined_from_property_access()to the condition, reusing the predicate this PR just hardened. Suggested tests alongside the new array-fold cases:ts.expectPrintedMin_("x = new ({f: a?.b}).f()", "x = new ({ f: a?.b }).f"); ts.expectPrintedMin_("({f: a?.b}).f`x`", "({ f: a?.b }).f`x`"); ts.expectPrintedMin_("x = new ({f: a?.[b]}).f()", "x = new ({ f: a?.[b] }).f");
Severity
Pre-existing. fold.rs is not touched by this PR and the behavior is unchanged from main. Flagging because it is the direct sibling of the fold being fixed, the output is a hard syntax error (worse than a semantic diff), and the fix is one line reusing this PR's own predicate — cheap to fold in here or file as an immediate follow-up.
…ead is_delete_target guard
The object-literal fold in maybe_rewrite_property_access returned the
value without consulting can_be_inlined_from_property_access, so
`new ({f: a?.b}).f()` minified to `new a?.b` (a hard SyntaxError) and
`({f: a?.b}).f`x`` minified to `a?.b`x``. Reuse the same predicate
there.
Drop the `!is_delete_target` clause from the array-fold guard:
`p.delete_target` is never set by the UnDelete arm so the clause was
dead, and the comment claimed `delete [x][0]` was protected when it is
not. That wiring is tracked separately.
robobun
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@test/bundler/transpiler/transpiler.test.js`:
- Around line 180-181: Remove the explanatory comments around the regression
test cases in the transpiler test, including the comment describing the sibling
predicate fold; retain only a comment containing the relevant issue URL if one
is available, otherwise leave those locations uncommented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b35a8cb5-3be4-4cb8-b5dc-893afbd0de4c
📒 Files selected for processing (3)
src/js_parser/fold.rssrc/js_parser/visit/visit_expr.rstest/bundler/transpiler/transpiler.test.js
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/js_parser/visit/visit_expr.rs (1)
1101-1105: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not replace sparse-array holes with
undefined.
EMissingrepresents an absent property. IfArray.prototype[0]exists,[,][0]reads that inherited value and can invoke a getter. This rewrite always emitsundefined, changing both results and side effects. Keep the indexed access for holes unless the minifier proves that indexedArray.prototypeproperties cannot be modified.🤖 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/js_parser/visit/visit_expr.rs` around lines 1101 - 1105, Remove the EMissing-to-E::Undefined rewrite in the indexed access handling around the inlined expression. Preserve the original indexed access for sparse-array holes, unless the minifier has an explicit proof that indexed Array.prototype properties cannot be modified; keep ordinary inlined values unchanged.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@src/js_parser/visit/visit_expr.rs`:
- Around line 1101-1105: Remove the EMissing-to-E::Undefined rewrite in the
indexed access handling around the inlined expression. Preserve the original
indexed access for sparse-array holes, unless the minifier has an explicit proof
that indexed Array.prototype properties cannot be modified; keep ordinary
inlined values unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4bce009a-bcca-44b6-b838-8a1abb0098e9
📒 Files selected for processing (1)
src/js_parser/visit/visit_expr.rs
There was a problem hiding this comment.
Thanks — 62cf6b3 addressed the dead !is_delete_target guard I flagged, and the PR body now scopes the p.delete_target / is_template_tag wiring gaps as pre-existing follow-ups. No new issues found this pass. Deferring to a maintainer to sign off on the scope split and the new (0, obj.m)() emission, since this is minifier-output correctness.
What was reviewed: the merged single/multi-item fold preserves the EMissing → undefined path and the can_be_inlined_from_property_access gate; the new (0, x)() emit matches the five sibling folds (comma/??/||/&&/ternary) exactly; the assign_target guard also covers the "foo"[n] = v string-index arm and [,][0] = v; the {f: x}.f fold's existing !is_call_target/!is_delete_target/assign_target == None gates mean the new predicate only adds the optional-chain bailout there.
Extended reasoning...
Overview
Four files: src/ast/expr.rs (three new arms in can_be_inlined_from_property_access rejecting EDot/EIndex/ECall with optional_chain set), src/js_parser/fold.rs (the {f:x}.f fold now consults that predicate), src/js_parser/visit/visit_expr.rs (the [x][0] fold gains an assign_target bailout, merges its two arms, turns the multi-item debug_assert! into a real guard, and emits (0, x)() when is_call_target && has_value_for_this_in_call()), plus ~120 lines of transpiler assertions and a runtime-semantics subprocess test.
Since my last review, 62cf6b3 dropped the dead && !is_delete_target clause I flagged (it could never fire because p.delete_target is never seeded by the UnDelete arm) and moved that fact into the PR description's out-of-scope section alongside the is_template_tag gap — both are pre-existing port gaps shared with the sibling folds.
Security risks
None. This is AST-level constant folding; the change strictly narrows when an existing rewrite fires. No untrusted-input parsing, allocation, or FFI is touched.
Level of scrutiny
High — minifier semantic correctness affects every file Bun transpiles under bun run (minify_syntax defaults on). The change is mostly a tightening (added bailouts, worst case = missed optimization), but the (0, obj.m)() emission is a new output shape. I verified it matches the exact pattern at visit_binary.rs:228/372/395/417 and visit_expr.rs:1502/1530, and has_value_for_this_in_call() (EDot | EIndex) is reached only after can_be_inlined_from_property_access() has already excluded optional-chain members, so the branch only fires for plain obj.m / obj[m]. The technical observable — this becomes undefined/global instead of the temp array — is the same trade the five sibling folds already make.
I also traced the merged single/multi-item logic against the old code: [,][0] still becomes undefined (the EMissing check now runs before the predicate, whereas before it fell through from the single-item arm to the multi-item arm), int == 0 is equivalent to the old number.value() == 0.0 under the enclosing integer/range guard, and the assign_target gate correctly sits above the string-index arm so "foo"[2] = 1 no longer becomes "o" = 1.
Other factors
Test coverage is thorough: exact-output assertions for chain start/continuation across . / [] / (), the multi-item @__PURE__ path, LHS/delete/new/tagged-template positions for the optional-chain bailout, call/assign/inc/destructuring for the new guards, plus positive cases proving the fold still fires. A subprocess runtime test cross-checks eleven cases against Node semantics. My earlier feedback was addressed; no new findings from the bug hunter this run.
I'm not approving because a maintainer should ratify the scope decision — leaving delete [obj.p][0] and [obj.m][0]x`` unfixed here is reasonable (wiring p.delete_target / `is_template_tag` touches every dormant guard and all sibling folds), but that's a call for a human, not a bot.
Two more parallel-arm sites in the same bug class:
- `BinNullishCoalescingAssign | BinLogicalOrAssign` in `visit_right_and_finish`
(the `{}.x ??= v` HMR fold) returned `e_.right` bare with no
`has_value_for_this_in_call()` guard. Fires without `minify_syntax`.
`({}.x ??= obj.m)()` / `({}.x ??= obj.m)\`t\`` now emit `(0, obj.m)`.
- `substitute_single_use_symbol_in_expr`'s `ETemplate` arm lacked the
'don't substitute into a target that could change this' check the parallel
`ECall` arm has. `let x = obj.m; x\`t\`` no longer inlines to
`obj.m\`t\``.
Both were pre-existing for `is_call_target` too and neither is covered by
#36730/#36734.
…in / this / assignment semantics (oven-sh#36730) ## Problem With `minify_syntax` on (the default for `bun run`), the `[x][0] -> x` and `{f: x}.f -> x` folds were replacing an index/property Reference with a bare value without fully checking the parent context. All of these reproduce against Node on current main: ```js var a = null; [[a?.b]][0]?.[0].c; // Node: TypeError Bun: undefined (output: a?.b.c) new ({f: a?.b}).f(); // Node: runs Bun: SyntaxError (output: new a?.b) var obj = { m() { return this === obj } }; [obj.m][0](); // Node: false Bun: true (output: obj.m()) var o = { p: 1 }; [o.p][0] = 5; o.p; // Node: 1 Bun: 5 (output: o.p = 5) "foo"[2] = 1; // Node: no-op Bun: SyntaxError (output: "o" = 1) ``` The direct form `[a?.b]?.[0].c` is hidden on main because the parse-time "drop `?.` on a non-nullable literal" simplification clears the outer chain first and the printer inserts `(a?.b)`; one level of indirection (`[[a?.b]][0]?.[0].c`, `({f:[a?.b]}).f?.[0].c`) bypasses that. ## Fix - `can_be_inlined_from_property_access` now rejects `EDot`/`EIndex`/`ECall` whose `optional_chain` is set, so an optional chain is never spliced onto a surrounding `?.` continuation and never lands as a bare `new` / template-tag target. - Both folds now route through that predicate (the `{f: x}.f` fold previously did not). - The `e_index` fold bails when `in_.assign_target != None`, so `[x][0] = v`, `"s"[n] = v` and `[,][0] = v` are left as written instead of producing `x = v` / `"s" = v` / `void 0 = v`. - When the `e_index` fold is a call target and the inlined item is an `EDot`/`EIndex`, it emits `(0, x)()` instead of `x()`, matching the sibling comma/`??`/`||`/`&&`/ternary folds so `this` is not rebound. - The multi-item path's `debug_assert!` on the predicate becomes a real guard (reachable via a `/* @__PURE__ */ a?.()` array item). The folds still fire for plain targets: `[a.b][0].c -> a.b.c`, `[[y]][0]?.[0].c -> y.c`, `[y][0]() -> y()`, `({f: y}).f -> y`, `new ({f: C}).f() -> new C`. esbuild does not perform these folds at all, so the remaining bailouts bring Bun in line with its output. ## Out of scope (tracked separately) Two pieces of parser-context tracking were never wired up in the Rust port, so the corresponding parent positions are not yet covered here for plain (non-chain) items: - `p.delete_target` is never set by the `UnDelete` arm, so `delete [obj.p][0]` still folds to `delete obj.p`. Wiring it up turns on every dormant `is_delete_target` guard in the visitor (bundler "cannot assign to import", CJS-named-export deopt, enum/namespace inlining under `delete`). - `is_template_tag` is commented out, so `[obj.m][0]`x`` still rebinds `this`. Wiring it up touches every sibling fold (comma/`??`/`||`/`&&`/ternary). Both are pre-existing and have a blast radius beyond this fold; the optional-chain bailout already keeps `delete [a?.b][0]` and `[a?.b][0]`x`` unfolded in the meantime. ## Verification - `bun bd test test/bundler/transpiler/transpiler.test.js -t "property access inlining"` passes (8 tests); the three new tests fail with `src/` reverted to main. - Full `transpiler.test.js` (186 pass / 0 fail) and `bundler_minify.test.ts` (42 pass / 0 fail) are green. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 4 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 3 failed, 22 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/transpiler/transpiler.test.js bun test v1.4.0 (aa68cff) test/bundler/transpiler/transpiler.test.js: (pass) Bun.Transpiler > handles errors when parsing macros [5.64ms] (pass) Bun.Transpiler > normalizes \r\n [5.92ms] 1 (pass) Bun.Transpiler > doesn't hang indefinitely oven-sh#2746 [3.87ms] (pass) Bun.Transpiler > property access inlining > bails out with spread [6.99ms] (pass) Bun.Transpiler > property access inlining > bails out with multiple items [2.54ms] (pass) Bun.Transpiler > property access inlining > works [2.17ms] (pass) Bun.Transpiler > property access inlining > works nested [2.49ms] 76 | transpiledOutput: code => { 77 | return ts.parsed(code, false, false); 78 | }, 79 | 80 | expectPrintedMin_: (code, out) => { 81 | expect(ts.parsedMin(code, !out.endsWith(";\n"), false)).toBe(out); ^ error: expect(received).toBe(expected) Expected: "x = [a?.b]?.[0].c" Received: "x = a?.b.c" at expectPrintedMin_ (/workspace/bun/test/bu ... (truncated) release without fix: 2 failed, 22 skipped bun test v1.4.0-canary.1 (8786d33) test/bundler/transpiler/transpiler.test.js: (pass) Bun.Transpiler > handles errors when parsing macros [2.42ms] (pass) Bun.Transpiler > normalizes \r\n [0.15ms] 1 (pass) Bun.Transpiler > doesn't hang indefinitely oven-sh#2746 [0.08ms] (pass) Bun.Transpiler > property access inlining > bails out with spread [0.14ms] (pass) Bun.Transpiler > property access inlining > bails out with multiple items [0.03ms] (pass) Bun.Transpiler > property access inlining > works [0.03ms] (pass) Bun.Transpiler > property access inlining > works nested [0.03ms] 76 | transpiledOutput: code => { 77 | return ts.parsed(code, false, false); 78 | }, 79 | 80 | expectPrintedMin_: (code, out) => { 81 | expect(ts.parsedMin(code, !out.endsWith(";\n"), false)).toBe(out); ^ error: expect(received).toBe(expected) Expected: "x = new { f: a?.b }.f" Received: "x = new a?.b" at expectPrintedMin_ (/workspace/bun/test/bundler/transpiler/transpiler.test.js:81:63) at <anonymous> (/workspace/bun/test/bundler/transpiler/transpiler.test.js:182:10) (fail) Bun.Transpiler > proper ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 22 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/transpiler/transpiler.test.js bun test v1.4.0 (aa68cff) test/bundler/transpiler/transpiler.test.js: (pass) Bun.Transpiler > handles errors when parsing macros [5.75ms] (pass) Bun.Transpiler > normalizes \r\n [6.06ms] 1 (pass) Bun.Transpiler > doesn't hang indefinitely oven-sh#2746 [4.03ms] (pass) Bun.Transpiler > property access inlining > bails out with spread [7.03ms] (pass) Bun.Transpiler > property access inlining > bails out with multiple items [2.56ms] (pass) Bun.Transpiler > property access inlining > works [2.25ms] (pass) Bun.Transpiler > property access inlining > works nested [2.63ms] (pass) Bun.Transpiler > property access inlining > bails out when the array item is an optional chain [45.16ms] (pass) Bun.Transpiler > property access inlining > bails out or strips `this` when the index is a call/assignment target [21.39ms] (pass) Bun.Transpiler > property access inlining > preserves runtime semantics when inlining from a literal index [324.61ms] (pass) Bun.Transpiler > property access inlining > bails out on optional ... (truncated) release with fix: 22 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 730ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/5] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli) �[1m�[92m Compiling�[0m bun_output v0.0.0 (/workspace/bun/src/output) �[1m�[92m Compiling�[0m bun_clap ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/ast/expr.rs | 5 ++ src/js_parser/fold.rs | 1 + src/js_parser/visit/visit_expr.rs | 36 +++++---- test/bundler/transpiler/transpiler.test.js | 118 +++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 15 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/ast/expr.rs 3 3 0 src/js_parser/fold.rs 2 1 0 src/js_parser/visit/visit_expr.rs 8 4 0 test/bundler/transpiler/transpiler.test.js 5 14 0 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2 participants
Problem
With
minify_syntaxon (the default forbun run), the[x][0] -> xand{f: x}.f -> xfolds were replacing an index/property Reference with a bare value without fully checking the parent context. All of these reproduce against Node on current main:The direct form
[a?.b]?.[0].cis hidden on main because the parse-time "drop?.on a non-nullable literal" simplification clears the outer chain first and the printer inserts(a?.b); one level of indirection ([[a?.b]][0]?.[0].c,({f:[a?.b]}).f?.[0].c) bypasses that.Fix
can_be_inlined_from_property_accessnow rejectsEDot/EIndex/ECallwhoseoptional_chainis set, so an optional chain is never spliced onto a surrounding?.continuation and never lands as a barenew/ template-tag target.{f: x}.ffold previously did not).e_indexfold bails whenin_.assign_target != None, so[x][0] = v,"s"[n] = vand[,][0] = vare left as written instead of producingx = v/"s" = v/void 0 = v.e_indexfold is a call target and the inlined item is anEDot/EIndex, it emits(0, x)()instead ofx(), matching the sibling comma/??/||/&&/ternary folds sothisis not rebound.debug_assert!on the predicate becomes a real guard (reachable via a/* @__PURE__ */ a?.()array item).The folds still fire for plain targets:
[a.b][0].c -> a.b.c,[[y]][0]?.[0].c -> y.c,[y][0]() -> y(),({f: y}).f -> y,new ({f: C}).f() -> new C. esbuild does not perform these folds at all, so the remaining bailouts bring Bun in line with its output.Out of scope (tracked separately)
Two pieces of parser-context tracking were never wired up in the Rust port, so the corresponding parent positions are not yet covered here for plain (non-chain) items:
p.delete_targetis never set by theUnDeletearm, sodelete [obj.p][0]still folds todelete obj.p. Wiring it up turns on every dormantis_delete_targetguard in the visitor (bundler "cannot assign to import", CJS-named-export deopt, enum/namespace inlining underdelete).is_template_tagis commented out, so[obj.m][0]x`` still rebindsthis. Wiring it up touches every sibling fold (comma/`??`/`||`/`&&`/ternary).Both are pre-existing and have a blast radius beyond this fold; the optional-chain bailout already keeps
delete [a?.b][0]and[a?.b][0]x`` unfolded in the meantime.Verification
bun bd test test/bundler/transpiler/transpiler.test.js -t "property access inlining"passes (8 tests); the three new tests fail withsrc/reverted to main.transpiler.test.js(186 pass / 0 fail) andbundler_minify.test.ts(42 pass / 0 fail) are green.[review] gate passed · iteration 1 · 4 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 1
evidence per changed file