Parse and lower accessor fields under experimentalDecorators - #29201
Closed
robobun wants to merge 3 commits into
Closed
Parse and lower accessor fields under experimentalDecorators#29201robobun wants to merge 3 commits into
accessor fields under experimentalDecorators#29201robobun wants to merge 3 commits into
Claude / Claude Code Review
completed
Aug 13, 2026 in 26m 32s
Code review found 1 important issue
Found 2 candidates, confirmed 4. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/js_parser/parse/mod.rs:287-288 |
Adding accessor under useDefineForClassFields:false silently flips sibling fields to [[Define]] semantics |
| 🟡 Nit | src/js_parser/parse/mod.rs:266-273 |
New compile error rejects tsc-valid accessor + legacy-decorator classes; gate is also over-broad (parameter decorators) |
Annotations
Check failure on line 288 in src/js_parser/parse/mod.rs
claude / Claude Code Review
Adding `accessor` under useDefineForClassFields:false silently flips sibling fields to [[Define]] semantics
Widening `should_lower_standard_decorators` to `has_auto_accessor || (...)` forces `use_define = true` at visit/mod.rs:1016-1017 for the whole class, so under `useDefineForClassFields: false` sibling `Normal` instance fields skip the `this.x = init` rewrite (:1044) and are emitted verbatim as in-body class fields by `lower_impl` (lower_decorators.rs:1666) — always [[Define]] semantics. In the canonical TypeORM/NestJS config (`experimentalDecorators: true` + `useDefineForClassFields: false`), `cl
Check warning on line 273 in src/js_parser/parse/mod.rs
claude / Claude Code Review
New compile error rejects tsc-valid accessor + legacy-decorator classes; gate is also over-broad (parameter decorators)
This error rejects tsc-valid code: TypeScript 4.9-5.9 compile both `class E { @Column() id = 0; accessor name = '' }` and `@dec accessor n = 1` under `experimentalDecorators: true` (private backing field + `__decorate` on the getter), so per REVIEW.md's "the reference implementation is the spec" this leaves #29197/#27335 unfixed for TypeORM/NestJS users. The gate is also over-broad: `has_any_decorators` folds in `opts.has_argument_decorators` (line 243), so `class C { constructor(@inject() x) {}
Loading