Skip to content

js_parser: stop the [x][0] / {f:x}.f folds from changing optional-chain / this / assignment semantics - #36730

Merged
Jarred-Sumner merged 6 commits into
mainfrom
farm/40497af5/array-index-inline-optional-chain
Aug 4, 2026
Merged

js_parser: stop the [x][0] / {f:x}.f folds from changing optional-chain / this / assignment semantics#36730
Jarred-Sumner merged 6 commits into
mainfrom
farm/40497af5/array-index-inline-optional-chain

js_parser: guard the [x][0] fold against call/assignment parent context

a1c5e68
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 1, 2026 in 34m 55s

Code review found 1 important issue

Found 4 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 0
🟣 Pre-existing 2
Severity File:Line Issue
🔴 Important src/js_parser/visit/visit_expr.rs:1065 !is_delete_target guard at :1065 is dead code — delete [obj.p][0] still folds
🟣 Pre-existing src/js_parser/visit/visit_expr.rs:1113-1122 Tagged-template tag position still rebinds this ([obj.m][0]x → obj.mx)

Annotations

Check failure on line 1065 in src/js_parser/visit/visit_expr.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

!is_delete_target guard at :1065 is dead code — delete [obj.p][0] still folds

The `!is_delete_target` guard is dead code — `p.delete_target` is never set to the operand in the `Op::UnDelete` arm (visit_expr.rs:1232-1234), so `is_delete_target` at :888 is always `false` and `delete [obj.p][0]` still folds to `delete obj.p`, contradicting the comment above. Add `p.delete_target = e_.value.data;` before the `visit_expr_in_out` call in `Op::UnDelete` (matching esbuild), and add `ts.expectPrintedMin_("delete [obj.p][0]", "delete [obj.p][0]")` plus a `delete` case in the runtim

Check notice on line 1122 in src/js_parser/visit/visit_expr.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Tagged-template tag position still rebinds this ([obj.m][0]`x` → obj.m`x`)

Pre-existing, fourth sibling of the call/delete/assign contexts the second commit guards here: `[obj.m][0]`tpl`` still folds to `obj.m`tpl``, rebinding `this` from the temp array to `obj` (Node prints `false` for `var obj={m(){return this===obj}}; [obj.m][0]`x``; the folded form prints `true`). `is_call_target` derives from `p.call_target` which only `e_call` sets (:1854); `e_template` (:695-696) visits the tag via plain `p.visit_expr()`, and `is_template_tag` is a commented-out TODO at :1044/:1