Let a decorated class body observe the class its decorators return - #38731
Let a decorated class body observe the class its decorators return#38731robobun wants to merge 9 commits into
Code review found 3 important issues
Found 4 candidates, confirmed 4. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 3 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/js_parser/lower/lower_decorators.rs:1765-1772 |
Relocated static fields containing super / new.target become a SyntaxError |
| 🔴 Important | src/js_parser/lower/lower_decorators.rs:2321-2331 |
rewrite_expr misses this in arrow defaults and object computed keys of relocated static fields |
| 🔴 Important | src/js_parser/lower/lower_decorators.rs:1350-1354 |
Hoisted computed keys of relocated static fields break source-order evaluation vs. in-body computed keys |
| 🟡 Nit | src/js_parser/visit/mod.rs:837-849 |
Class-name references in extends / non-hoisted computed keys resolve to undefined instead of hitting the TDZ |
Annotations
Check failure on line 1772 in src/js_parser/lower/lower_decorators.rs
claude / Claude Code Review
Relocated static fields containing `super` / `new.target` become a SyntaxError
Relocating an undecorated static field whose initializer uses `super` (or `new.target`) now emits it verbatim as a `__publicField(...)` argument at the enclosing scope, which is a hard SyntaxError. Before this PR such fields stayed in the class body and ran; `is_plain_static_field` needs a bail-out (like the `has_private_members` one) when the initializer contains `super`/`new.target`, since `rewrite_expr` has no way to rewrite those.
Check failure on line 2331 in src/js_parser/lower/lower_decorators.rs
claude / Claude Code Review
rewrite_expr misses `this` in arrow defaults and object computed keys of relocated static fields
`rewrite_expr` doesn't recurse into arrow parameter defaults (`EArrow` walks only `body.stmts`, not `args`) or object computed keys (`EObject` walks `value`/`initializer`, not `key`), so `@wrap class Foo { static make = (x = this) => x; static t = { [this.name]: 1 } }` now emits `__publicField(Foo, "make", (x = this) => x)` at statement scope where `this === undefined` — `Foo.make()` returns `undefined` and `[this.name]` throws, whereas before this PR the field stayed in the body and `this` was
Check failure on line 1354 in src/js_parser/lower/lower_decorators.rs
claude / Claude Code Review
Hoisted computed keys of relocated static fields break source-order evaluation vs. in-body computed keys
Widening the pre-eval condition to hoist computed keys of relocated undecorated static fields breaks source-order evaluation when they're interleaved with computed keys of members that stay in the body (undecorated instance fields, methods, getters/setters). `@wrap class Foo { static [key("s1")] = 1; [key("i")] = 2; static [key("s2")] = 3; }` now evaluates keys as `["s1","s2","i"]` instead of the spec-correct `["s1","i","s2"]` that bun 1.4.0 produced — the PR's own "in source order" test only in
Check warning on line 849 in src/js_parser/visit/mod.rs
claude / Claude Code Review
Class-name references in extends / non-hoisted computed keys resolve to `undefined` instead of hitting the TDZ
Redirecting the ClassName scope to `_Foo` also captures the `extends` clause and computed keys of *undecorated* members — those evaluate during ClassDefinitionEvaluation before `static { _Foo = this }` runs, so they now see `undefined` instead of the spec's TDZ ReferenceError. E.g. `@wrap class Foo { [String(Foo)]() {} }` used to throw `ReferenceError` (correct) but now silently defines a method keyed `"undefined"`; `@wrap class Foo extends mixin(Foo) {}` now calls `mixin(undefined)`. Only affec