Repository navigation
feat(transpiler): implement TC39 standard ES decorators lowering - #26436
Merged
Merged
Conversation
Collaborator
|
Updated 10:40 PM PT - Feb 9th, 2026
❌ @Jarred-Sumner, your commit cbeeec9 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 26436That installs a local version of the PR into your bun-26436 --bun |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds TC39-standard decorator parsing and lowering, runtime decorator helpers/exports, class auto-accessor parsing/kind, plumbing for an experimental_decorators flag across resolver/transpiler/bundler/parse, and propagation of decorator binding names through AST visitors; includes tests and lexer/printer updates. Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/ast/visitBinaryExpression.zig`:
- Around line 73-79: The code currently unconditionally clears
p.decorator_class_name causing outer decorator name scopes to be lost; modify
the handling around the conditional that assigns p.decorator_class_name (the
block checking e_.op == .bin_assign, was_anonymous_named_expr, e_.right.data ==
.e_class and e_.right.data.e_class.should_lower_standard_decorators,
e_.left.data == .e_identifier) to save the previous value into a temporary
(e.g., old = p.decorator_class_name), set p.decorator_class_name =
p.loadNameFromRef(e_.left.data.e_identifier.ref) while processing the inner
expression, and then restore p.decorator_class_name = old afterwards; apply the
same save/restore pattern at the other occurrence referenced in the comment
(around the second similar assignment).
In `@src/ast/visitStmt.zig`:
- Around line 216-223: The code sets p.decorator_class_name for anonymous class
decorators then unconditionally clears it, which can clobber a prior value;
modify the .expr arm so you save the previous value of p.decorator_class_name
into a temp, set the new value when needed (based on expr.isAnonymousNamed() &&
expr.data == .e_class && expr.data.e_class.should_lower_standard_decorators),
call p.visitExpr(expr) to populate data.value.expr, and then restore the saved
temp back to p.decorator_class_name (use a defer or explicit restore to ensure
restoration even if visitExpr fails); reference p.decorator_class_name,
visitExpr, expr.isAnonymousNamed, and
expr.data.e_class.should_lower_standard_decorators to locate the change.
In `@src/bundler/ParseTask.zig`:
- Around line 1189-1191: Update the decorator feature flags to use the per-file
settings on the task instead of the global transpiler options: replace uses of
transpiler.options.emit_decorator_metadata and
transpiler.options.experimental_decorators when setting
opts.features.emit_decorator_metadata and opts.features.standard_decorators so
they read the corresponding fields on task (e.g., task.emit_decorator_metadata
and task.experimental_decorators) while keeping the existing
loader.isTypeScript() check for standard_decorators logic.
In `@src/runtime.js`:
- Around line 241-246: The setter currently uses an unnecessary return: replace
the body of the computed setter (set [name](x) { return __privateSet(this,
extra, x); }) with a void call (set [name](x) { __privateSet(this, extra, x); })
so it does not return a value; update the setter that references __privateSet
and extra to simply call __privateSet(this, extra, x) without returning it to
eliminate the redundant return and static-analysis warnings.
- Line 204: The sparse array [, , , __create(base?.[__knownSymbol("metadata")]
?? null)] in __decoratorStart is intentional to reserve specific initializer
slots for the TC39 decorator runtime but looks suspicious to static analyzers;
add a brief inline comment above or next to the expression in function
__decoratorStart explaining that the empty slots correspond to initializer
indices (e.g., placeholders for field/method/class initializers) and that
__create is placed in the fourth slot using the knownSymbol("metadata") lookup,
referencing __create, __knownSymbol, and "metadata" so future maintainers and
linters understand the pattern.
| ); | ||
| export var __privateMethod = (obj, member, method) => (__accessCheck(obj, member, "access private method"), method); | ||
|
|
||
| export var __decoratorStart = base => [, , , __create(base?.[__knownSymbol("metadata")] ?? null)]; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Sparse array is intentional but could benefit from a brief comment.
The static analyzer flags [, , , __create(...)] as suspicious, but this sparse array is an intentional pattern for TC39 decorator runtimes where indices represent different initializer types. Consider adding a brief comment to clarify the index semantics for future maintainers.
📝 Suggested documentation
-export var __decoratorStart = base => [, , , __create(base?.[__knownSymbol("metadata")] ?? null)];
+// Indices: [0]=class init, [1]=static member init, [2]=instance member init, [3]=metadata
+export var __decoratorStart = base => [, , , __create(base?.[__knownSymbol("metadata")] ?? null)];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export var __decoratorStart = base => [, , , __create(base?.[__knownSymbol("metadata")] ?? null)]; | |
| // Indices: [0]=class init, [1]=static member init, [2]=instance member init, [3]=metadata | |
| export var __decoratorStart = base => [, , , __create(base?.[__knownSymbol("metadata")] ?? null)]; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 204-204: This array contains an empty slots..
The presences of empty slots may cause incorrect information and might be a typo.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
🤖 Prompt for AI Agents
In `@src/runtime.js` at line 204, The sparse array [, , ,
__create(base?.[__knownSymbol("metadata")] ?? null)] in __decoratorStart is
intentional to reserve specific initializer slots for the TC39 decorator runtime
but looks suspicious to static analyzers; add a brief inline comment above or
next to the expression in function __decoratorStart explaining that the empty
slots correspond to initializer indices (e.g., placeholders for
field/method/class initializers) and that __create is placed in the fourth slot
using the knownSymbol("metadata") lookup, referencing __create, __knownSymbol,
and "metadata" so future maintainers and linters understand the pattern.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/ast/lowerDecorators.zig`:
- Around line 701-770: The code treats any private non-decorated member as a
field when lower_all_private is true, which incorrectly lowers private
auto-accessors; update the branch in the loop over class.properties (the block
guarded by lower_all_private and prop.key?.data == .e_private_identifier) to
detect prop.kind == .auto_accessor and route those to the auto-accessor lowering
logic (the same path that starts at the dedicated auto-accessor handler after
this block) instead of the WeakMap field path: e.g., check prop.kind ==
.auto_accessor before creating a WeakMap (wm_ref) and calling
emitPrivateAdd/__privateAdd and, when detected, invoke the auto-accessor
lowering routine (ensuring private_lowered_map, constructor_inject_stmts,
static_private_add_blocks, and emitted_private_adds are updated consistently).
Ensure you do not duplicate behavior for methods (methodKind, fnSuffix) and keep
existing handling for .is_method unchanged.
- Around line 1312-1328: The helper appendDeclsAsAssigns currently only
processes .s_local and thus drops .s_expr statements when lowering prefix_stmts
and pre_eval_stmts in expression mode; update appendDeclsAsAssigns (in
src/ast/lowerDecorators.zig) to check for pstmt.data == .s_expr before the
.s_local branch and append the expression statement's inner expression to parts
(using the parser/new Expr construction consistent with Expr.assign usage),
ensuring any required parser.recordUsage or other bookkeeping for that
expression is performed so side-effecting statements are preserved in
expression-mode lowering.
- Around line 95-103: The newWeakSetExpr function creates an unbound symbol
using newSym which bypasses lexical scope; update it to mirror newWeakMapExpr by
using findSymbol to resolve the "WeakSet" identifier in the current scope
(instead of newSym(..., .unbound, "WeakSet")), so replace the newSym call with a
call to findSymbol(p, "WeakSet") and use that result as the ref when
constructing the Identifier Expr.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@test/bundler/transpiler/es-decorators-esbuild.test.ts`:
- Around line 26-27: Remove the unused boilerplate variables _testName and
_failures from the test file (they are declared at the top of
es-decorators-esbuild.test.ts but never referenced); either delete their
declarations or, if they are intended for future use, add a short comment
explaining their purpose and expected usage so the linter/test runner won’t flag
them as unused. Ensure references to _testName and _failures do not exist
elsewhere before removing.
- Around line 67-72: The spawn call for proc created by Bun.spawn does not
explicitly request piping stdout even though the test later calls
proc.stdout.text(); update the Bun.spawn options in the test (the block that
constructs proc in es-decorators-esbuild.test.ts) to include stdout: "pipe"
alongside stderr: "pipe" so proc.stdout is guaranteed to be available when
proc.stdout.text() is invoked.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/bundler/bundler_promiseall_deadcode.test.ts (1)
74-166: Stabilize snapshot output withnormalizeBunSnapshot.
These debugId-only updates are noise; wrap the received string innormalizeBunSnapshot(...)beforetoMatchInlineSnapshotto reduce churn. As per coding guidelines, ...
|
so happy to see this PR alive; just found a parsing bug when using decorators with the |
|
❌ encountered a crash with /opt/homebrew/bin/bun-26436 --eval "
function dec(target, context) {
return function(...args) {
console.log('decorator intercepted:', context.name)
return target.apply(this, args)
}
}
export default class Foo {
@dec
method() { return 42 }
}
const result = new Foo().method()
console.log('result:', result)"Bun Canary v1.3.7-canary.1 (69e77113) macOS Silicon
macOS v15.7.3
CPU: fp aes crc32 atomics
Args: "/opt/homebrew/bin/bun-26436" "--eval" "\nfunction dec(target, context) {\n return function(...args) {\n console.log('decorator intercepted:', context.name)\n return target.apply(this, args)\n }\n}\nexp"...
Features: debugger jsc
Builtins: "bun:main"
Elapsed: 7ms | User: 10ms | Sys: 11ms
RSS: 24.07MB | Peak: 24.05MB | Commit: 0B | Faults: 46 | Machine: 34.36GB
panic(main thread): Unexpected type in export default
Crashed while visiting //[eval]
oh no: Bun has crashed. This indicates a bug in Bun, not your code. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/ast/P.zig`:
- Around line 494-497: The field decorator_class_name is currently public but
intended internal-only; rename it to `#decorator_class_name` and update any direct
references and accessor usage accordingly (e.g., reads/writes in visitExpr and
lowerStandardDecoratorsImpl) to use the new private-field name, and adjust any
getters/setters or patterns that access it so they compile with Zig's private
field syntax.
In `@src/ast/visitStmt.zig`:
- Around line 469-486: The code silently swallows allocation failures when
emitting class-related statements: replace the empty error handlers with hard
failures so allocation errors aren't ignored — change the two
stmts.appendSlice(class_stmts[0..class_stmt_idx]) catch {} and
stmts.append(stmt.*) catch {} and the trailing
stmts.appendSlice(class_stmts[class_stmt_idx + 1 ..]) catch {} to use catch
unreachable instead; keep using class_stmts, class_stmt_idx and
stmts.appendSlice/append as-is so the statements still emit in the same order
but now consistently propagate allocation failures like the rest of this file.
In `@test/bundler/transpiler/es-decorators.test.ts`:
- Around line 23-28: The Bun.spawn call that creates proc (variable name proc in
the test) only sets stderr: "pipe" but the test later reads proc.stdout.text();
update the Bun.spawn invocation (the object passed to Bun.spawn where
cmd/env/cwd/stderr are set) to also include stdout: "pipe" so stdout is
explicitly piped and intent matches stderr handling.
Comment on lines
+494
to
+497
| /// Name from assignment context for anonymous decorated class expressions. | ||
| /// Set before visitExpr, consumed by lowerStandardDecoratorsImpl. | ||
| decorator_class_name: ?[]const u8 = null, | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider private field prefix if this is internal-only
If decorator_class_name isn’t part of the public parser state API, prefer #decorator_class_name and adjust accessors accordingly.
As per coding guidelines: Use the # prefix for private fields in Zig structs, e.g., struct { #foo: u32 };
🤖 Prompt for AI Agents
In `@src/ast/P.zig` around lines 494 - 497, The field decorator_class_name is
currently public but intended internal-only; rename it to `#decorator_class_name`
and update any direct references and accessor usage accordingly (e.g.,
reads/writes in visitExpr and lowerStandardDecoratorsImpl) to use the new
private-field name, and adjust any getters/setters or patterns that access it so
they compile with Zig's private field syntax.
Comment on lines
+23
to
+28
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "test.js"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add explicit stdout: "pipe" for consistency and clarity.
The code reads from proc.stdout.text() on line 30 but only explicitly sets stderr: "pipe". While Bun's spawn may handle this correctly, explicitly setting stdout: "pipe" makes the intent clear and matches the stderr handling.
Suggested fix
await using proc = Bun.spawn({
cmd: [bunExe(), "test.js"],
env: bunEnv,
cwd: String(dir),
+ stdout: "pipe",
stderr: "pipe",
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), "test.js"], | |
| env: bunEnv, | |
| cwd: String(dir), | |
| stderr: "pipe", | |
| }); | |
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), "test.js"], | |
| env: bunEnv, | |
| cwd: String(dir), | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| }); |
🤖 Prompt for AI Agents
In `@test/bundler/transpiler/es-decorators.test.ts` around lines 23 - 28, The
Bun.spawn call that creates proc (variable name proc in the test) only sets
stderr: "pipe" but the test later reads proc.stdout.text(); update the Bun.spawn
invocation (the object passed to Bun.spawn where cmd/env/cwd/stderr are set) to
also include stdout: "pipe" so stdout is explicitly piped and intent matches
stderr handling.
Implement complete lowering of TC39 stage-3 standard decorators (the non-legacy variant used when tsconfig has no `experimentalDecorators`). Passes all 147 esbuild decorator tests plus 22 additional Bun-specific decorator tests. Features implemented: - Method, getter, setter decorators (static and instance) - Field decorators with initializer replacement and extra initializers - Auto-accessor lowering (accessor keyword → WeakMap + getter/setter) - Private member decorators (methods, fields, accessors via WeakMap/WeakSet) - Class decorators (statement and expression positions) - Class expression decorators with comma-expression lowering - Decorator metadata (Symbol.metadata) - Proper decorator evaluation order (source order per TC39 spec) - Class binding semantics (inner vs outer class name bindings) - Static block extraction with this-replacement - Computed property key pre-evaluation Runtime helpers added to src/runtime.js: __decoratorStart, __decorateElement, __decoratorMetadata, __runInitializers Fixes #4122 Fixes #20206 Fixes #14529 Fixes #6051 Co-Authored-By: Claude <noreply@anthropic.com>
….zig Move ~2027 lines of ES decorator lowering logic from P.zig into a dedicated lowerDecorators.zig module. Key improvements: - Helper functions (useRef, callRt, newSym) eliminate duplicated allocation and recordUsage patterns (~80 sites consolidated) - Unified tree rewriter replaces 4 separate replace functions - Private access rewriting uses one-liner helpers instead of manual arg-array construction - Net reduction of ~600 lines (2027 removed, 1426 added) All 191 decorator tests pass (es-decorators-esbuild: 147, es-decorators: 22, decorators: 22). Co-Authored-By: Claude <noreply@anthropic.com>
The vendor/esbuild submodule isn't available in CI. Copy decorator-tests.ts into test/bundler/transpiler/ and update the import path. Co-Authored-By: Claude <noreply@anthropic.com>
- Save/restore decorator_class_name instead of hard-resetting to null in visitBinaryExpression.zig and visitStmt.zig to avoid clobbering outer decorator name scopes in nested contexts - Use per-file task.emit_decorator_metadata and task.experimental_decorators instead of global transpiler.options in ParseTask.zig so per-file tsconfig settings are respected - Make newWeakSetExpr use findSymbol for proper lexical scope resolution, consistent with newWeakMapExpr - Exclude auto_accessor from private field lowering path so private auto-accessors get proper getter/setter pairs in decorated classes - Handle S.SExpr statements in appendDeclsAsAssigns so side-effect statements are preserved in expression-mode decorator lowering - Remove unnecessary return from setter in runtime.js Co-Authored-By: Claude <noreply@anthropic.com>
Remove unused _testName/_failures variables from test boilerplate and add explicit stdout: "pipe" to Bun.spawn call. Co-Authored-By: Claude <noreply@anthropic.com>
The debugId hash changed due to the runtime.js setter fix. Co-Authored-By: Claude <noreply@anthropic.com>
…tests - Update debugId snapshots in cyclic-imports-async-bundler.test.js and bundler_promiseall_deadcode.test.ts (changed by runtime.js setter fix) - Add experimentalDecorators: true tsconfig to TypeScriptDecoratorsSimpleCase and TypeScriptDecoratorScopeESBuildIssue2147 tests which expect legacy decorator behavior (now that standard decorators are default for TS) Co-Authored-By: Claude <noreply@anthropic.com>
…g with decorators Fix two bugs in ES decorator lowering: 1. `export default class` with decorators crashed because the code assumed the first statement from lowerClass was always s_class, but standard decorator lowering can produce prefix variable declarations. 2. `accessor field!: Type` (definite assignment assertion) failed to parse because the `!` operator was only recognized for `kind == .normal`, excluding `auto_accessor`. Also fix DCE check panic on non-s_class statements in export default. Co-Authored-By: Claude <noreply@anthropic.com>
Jarred-Sumner
force-pushed
the
jarred/esdecorator
branch
from
January 28, 2026 21:50
9eaf9fb to
0131156
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ast/G.zig (1)
45-69: UpdateClass.canBeMovedto treat static auto‑accessors like fieldsWith
Property.Kind.auto_accessor, static auto‑accessor initializers run during class evaluation and can have side effects.Class.canBeMovedcurrently only checks.normal, so it may incorrectly allow moves that reorder those side effects. Treat.auto_accessorthe same as.normalin the static initializer check.🐛 Proposed fix
- if (property.kind == .normal) { + if (property.kind == .normal or property.kind == .auto_accessor) { if (flags.contains(.is_static)) { for ([2]?Expr{ property.value, property.initializer }) |val_| { if (val_) |val| { switch (val.data) { .e_arrow, .e_function => {}, else => { if (!val.canBeMoved()) { return false; } }, } } } } }
🤖 Fix all issues with AI agents
In `@src/ast/lowerDecorators.zig`:
- Around line 236-238: The .e_template arm in lowerDecorators.zig currently only
rewrites e.tag and therefore skips rewriting the template parts and embedded
expressions; update the .e_template match to iterate over the template's
parts/segments (and any head/tail expressions) and call rewriteExpr(p, partExpr,
kind) for each embedded expression so replace_ref/replace_this runs inside
`${...}` and template substitutions are rewritten accordingly—look for the
.e_template case and the template structure fields (e.parts, e.head,
e.expressions or similarly named members) and invoke rewriteExpr for each.
- Around line 1113-1120: The current .block lowering iterates
extracted_static_blocks.items[elem.index] and only pulls out .s_expr statements
into suffix_exprs, dropping other statements; fix this by lowering the entire
sb.stmts into an IIFE (preserving let/if/for/try) and appending that resulting
expression to suffix_exprs instead of only collecting .s_expr entries: use the
existing rewriteStmts call on sb.stmts.slice() to transform the statements, then
wrap the transformed sb.stmts into a block-expression/IIFE (so all statements
execute and any final value becomes the expression) and push that single
expression into suffix_exprs; update the .block arm that references
extracted_static_blocks, sb.stmts, rewriteStmts, and suffix_exprs accordingly so
no non-expression statements are discarded.
In `@src/ast/P.zig`:
- Around line 4872-4875: The code path for "export default class" with standard
decorators can panic instead of routing through the standard decorator lowering;
ensure that when a statement is an exported default class whose
should_lower_standard_decorators flag is set, it returns
p.lowerStandardDecoratorsStmt(stmt) (use the existing check of
stmt.data.s_class.class.should_lower_standard_decorators and the
lowerStandardDecoratorsStmt function) and does not take any alternative path
that can panic, and add/verify a regression test that parses/transforms an
`export default class` with at least one standard decorator to confirm the
lowering occurs without panic.
In `@src/ast/parseProperty.zig`:
- Around line 302-310: The parser currently allows method syntax to slip through
for auto-accessor fields because kind == .auto_accessor bypasses the normal
field-gate; add an explicit syntax error check immediately before the
method-parsing gate (where method parsing is considered when kind != .normal)
that detects auto-accessor + method forms and raises a parse error (e.g.,
"auto-accessor fields cannot be methods") instead of falling through:
specifically, when kind == .auto_accessor and the upcoming tokens indicate a
method form (presence of function-like tokens such as '(', an async modifier
before the accessor, or a '*' generator marker), emit the syntax error and stop
parsing as a method. Apply the same guard/early-error in the other spots that
handle auto_accessor in the TypeScript metadata and field-parsing conditions
(the regions around the .auto_accessor checks at the other referenced locations)
so the invalid forms are consistently rejected.
In `@src/ast/visit.zig`:
- Around line 189-196: The visitor code sets p.decorator_class_name to a new
value before calling p.visitExprInOut and then unconditionally clears it with
p.decorator_class_name = null, which clobbers any outer value; change each such
block (e.g., the block using decl.binding, p.loadNameFromRef, and
p.visitExprInOut) to save the previous value into a temporary (prev =
p.decorator_class_name), assign the new name, call p.visitExprInOut, and then
restore p.decorator_class_name = prev; apply this pattern to the other
occurrences you noted (around the blocks at the other line ranges) so nested
visits correctly preserve outer decorator_class_name.
Comment on lines
+4872
to
+4875
| // Standard decorator lowering path (for both JS and TS files) | ||
| if (stmt.data.s_class.class.should_lower_standard_decorators) { | ||
| return p.lowerStandardDecoratorsStmt(stmt); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Please add/verify a regression for export default class with standard decorators.
A crash for this case was reported; ensure this path routes through lowerStandardDecoratorsStmt without panicking.
🤖 Prompt for AI Agents
In `@src/ast/P.zig` around lines 4872 - 4875, The code path for "export default
class" with standard decorators can panic instead of routing through the
standard decorator lowering; ensure that when a statement is an exported default
class whose should_lower_standard_decorators flag is set, it returns
p.lowerStandardDecoratorsStmt(stmt) (use the existing check of
stmt.data.s_class.class.should_lower_standard_decorators and the
lowerStandardDecoratorsStmt function) and does not take any alternative path
that can panic, and add/verify a regression test that parses/transforms an
`export default class` with at least one standard decorator to confirm the
lowering occurs without panic.
Comment on lines
+302
to
+310
| .p_accessor => { | ||
| // "accessor" keyword for auto-accessor fields (TC39 standard decorators) | ||
| if (opts.is_class and p.options.features.standard_decorators and | ||
| (js_lexer.PropertyModifierKeyword.List.get(raw) orelse .p_static) == .p_accessor) | ||
| { | ||
| kind = .auto_accessor; | ||
| errors = null; | ||
| continue :restart; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TC39 decorators auto-accessor proposal valid syntax rules methods
💡 Result:
Where decorators are valid (in the current TC39 decorators + auto-accessors design)
Decorators are syntactically allowed only on classes and class elements (not on object literals, functions, params, etc.). In the spec grammar, a DecoratorList may appear before MethodDefinition, FieldDefinition, class declarations, and class expressions. [3]
Concretely, the Stage 3 proposal supports decorating: classes, fields, methods, accessors, and “auto-accessors”. [1]
Valid decorator expression syntax (what can appear after @)
Decorator expressions are intentionally restricted:
- Identifier / dotted property access with
. - Calls
() - No computed property access (
[]) - To use an arbitrary expression, use the escape hatch:
@(expression)[1]
Example of the restriction (explicitly called out in the proposal text):
@foo.bar()✅@foo[bar]❌@foo().bar()❌ [4]
(Those restrictions correspond to the spec’s productions like @ DecoratorMemberExpression and @ DecoratorCallExpression, where member access is via . and calls apply to the member expression.) [3]
Decorating methods (valid forms)
Because decorators attach to MethodDefinition in a class body [3], these are all valid forms:
class C {
`@dec` m() {}
`@dec` static sm() {}
`@dec` `#pm`() {}
}Methods are explicitly a supported decoration target in the proposal. [1]
Auto-accessors: what they are + valid syntax
An auto-accessor is a new class element form written by putting the accessor keyword on what looks like a field declaration:
class C {
`@dec` accessor x = 1;
`@dec` static accessor y = 2;
`@dec` accessor `#z` = 3;
}Auto-accessors are explicitly defined as “adding the accessor keyword in front of a class field”, and they can be decorated. [2]
🏁 Script executed:
# Locate and examine the file mentioned in the review
find . -name "parseProperty.zig" -type fRepository: oven-sh/bun
Length of output: 82
🏁 Script executed:
# Once we find the file, let's examine the relevant sections
# First, let's get an overview of the file structure
git ls-files | grep -i parsePropertyRepository: oven-sh/bun
Length of output: 80
🏁 Script executed:
# Read the relevant sections of parseProperty.zig
wc -l src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 84
🏁 Script executed:
# Read around line 302-310
sed -n '280,330p' src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 3583
🏁 Script executed:
# Read around line 422-425
sed -n '400,450p' src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 2493
🏁 Script executed:
# Look for method parsing to understand the flow better
sed -n '430,500p' src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 3743
🏁 Script executed:
# Also check earlier to understand the flow - look for parseMethodExpression or similar
sed -n '450,550p' src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 4933
🏁 Script executed:
# Get more context - look for "parseMethodExpression" to understand method parsing flow
sed -n '520,530p' src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 471
🏁 Script executed:
# Search for other occurrences of kind == .auto_accessor or auto_accessor in the file
rg -n "auto_accessor" src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 300
🏁 Script executed:
# Get full context around line 520-525 to understand exact fix location
sed -n '515,530p' src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 673
🏁 Script executed:
# Check what's at lines 422-425 specifically
sed -n '420,428p' src/ast/parseProperty.zigRepository: oven-sh/bun
Length of output: 546
Reject method syntax for accessor fields.
Auto-accessor fields (TC39 class feature) must be field declarations only. Syntax like accessor foo(), async accessor, or accessor *foo() is invalid. Currently, when the field-parsing gate fails for these cases, the code falls through to method parsing because the method gate condition kind != .normal evaluates to true for kind == .auto_accessor. Add an explicit syntax error before method parsing to catch these invalid forms.
🐛 Suggested guard for invalid accessor method syntax
+ // Auto-accessors are fields, not methods
+ if (opts.is_class and kind == .auto_accessor and (opts.is_async or opts.is_generator or p.lexer.token == .t_open_paren)) {
+ p.log.addRangeError(p.source, key_range, "Auto-accessor fields cannot be methods") catch unreachable;
+ return error.SyntaxError;
+ }
+
// Parse a method expression
if (p.lexer.token == .t_open_paren or kind != .normal or opts.is_class or opts.is_async or opts.is_generator) {
return parseMethodExpression(p, kind, opts, is_computed, &key, key_range);
}Also applies to: 422–425, 441–444 (where auto_accessor kind handling appears in TypeScript type metadata and field-parsing conditions; ensure consistency with this guard).
🤖 Prompt for AI Agents
In `@src/ast/parseProperty.zig` around lines 302 - 310, The parser currently
allows method syntax to slip through for auto-accessor fields because kind ==
.auto_accessor bypasses the normal field-gate; add an explicit syntax error
check immediately before the method-parsing gate (where method parsing is
considered when kind != .normal) that detects auto-accessor + method forms and
raises a parse error (e.g., "auto-accessor fields cannot be methods") instead of
falling through: specifically, when kind == .auto_accessor and the upcoming
tokens indicate a method form (presence of function-like tokens such as '(', an
async modifier before the accessor, or a '*' generator marker), emit the syntax
error and stop parsing as a method. Apply the same guard/early-error in the
other spots that handle auto_accessor in the TypeScript metadata and
field-parsing conditions (the regions around the .auto_accessor checks at the
other referenced locations) so the invalid forms are consistently rejected.
- Fix e_template rewriting to iterate over template parts, not just tag
- Fix static block lowering to wrap non-expression stmts in IIFE
instead of silently dropping let/if/for/try statements
- Add parse error for `accessor foo() {}` (auto-accessor as method)
- Save/restore decorator_class_name in visit.zig instead of clobbering
outer value with null
Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/ast/lowerDecorators.zig`:
- Around line 255-277: rewriteStmts currently only rewrites expressions, locals,
and returns while rewritePrivateAccessesInStmts covers control-flow statements;
unify them by extracting a shared statement-walking helper (e.g., rewriteStmt or
rewriteStmtsCore) or simply have rewriteStmts delegate to
rewritePrivateAccessesInStmts so all variants (.s_if, .s_for, .s_while,
.s_switch, .s_try, etc.) are visited; ensure the helper recurses into condition,
body, else/elif branches, loop init/cond/incr, switch cases, try/catch/finally
blocks and still applies expression rewriting via rewriteExpr where appropriate
(referencing functions rewriteStmts, rewritePrivateAccessesInStmts, and
rewriteExpr to locate code).
- Around line 255-277: The rewriteStmts function currently only handles .s_expr,
.s_local, and .s_return and thus misses control-flow branches where `this` may
appear; update rewriteStmts (and use rewriteExpr) to cover .s_if, .s_for,
.s_while, .s_do, .s_switch (and any other statement variants in the Stmt union)
by recursively visiting their condition expressions and all nested statement
lists/blocks so `rewriteExpr` runs on every contained Expr; ensure you also
handle else/elif/initializer/update bodies and switch case bodies so no
control-flow path escapes the `this`→class-name replacement logic.
- Line 626: The line setting original_class_name_for_decorator currently uses
direct field access class_name_ref.inner_index; change it to use the Ref
accessor method class_name_ref.innerIndex() to match the rest of the file and to
ensure consistent/bounds-checked access into p.symbols.items (the same pattern
used elsewhere like at innerIndex() call sites). Update the assignment that
reads p.symbols.items[class_name_ref.inner_index].original_name to use
p.symbols.items[class_name_ref.innerIndex()].original_name so it follows the
established access convention.
…accessor - Add all control-flow statement cases to rewriteStmts (s_if, s_for, s_for_in, s_for_of, s_while, s_do_while, s_switch, s_try, s_block, s_label, s_with, s_throw) mirroring rewritePrivateAccessesInStmts, so this→className replacement works inside if/for/while/switch/try in static blocks and other rewritten contexts - Replace all .inner_index field accesses with .innerIndex() method calls for consistent bounds-checked access into p.symbols.items Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/ast/lowerDecorators.zig`:
- Around line 1313-1335: The current constructor injection only looks for
top-level super() by scanning func.func.body.stmts, so when super() is nested in
control-flow the code falls back to inserting at 0; add a recursive search that
returns the index of the enclosing top-level statement that contains a super()
call (so we insert after that statement) — implement a helper (e.g.
findEnclosingStmtIndexWithSuper(stmts: []Stmt) -> ?usize) that iterates
statements: if a statement is an expression-call-to-super return its index,
otherwise for composite statements (s_if, s_block, s_while, s_for, s_switch,
s_try, etc.) recurse into their inner stmt lists and if recursion finds super
return the outer statement's index; replace the existing top-level loop that
computes super_index with a call to this helper and keep the existing insert_at
= if (super_index) |j| j + 1 else 0 and insertion via
body_stmts.insertSlice(...) so decorator initializers are injected after the
correct enclosing statement.
- Around line 1160-1187: The extracted static-block handling currently always
moves blocks to suffixes (wrapping non-expression statements in an Arrow IIFE)
which breaks `super` semantics; update the logic that processes
extracted_static_blocks.items[elem.index] (used with rewriteStmts, suffix_exprs,
p.newExpr, E.Arrow, E.Call and class_name_ref/class_name_loc) to first scan
sb.stmts for any Super nodes; if any `super` reference is present, do not
extract/move the block — instead preserve the original static block inside the
class body; alternatively (if you prefer rewriting), implement a rewrite in
rewriteStmts that transforms `super` property gets/sets/calls into equivalent
Reflect.get/Reflect.set or explicit function call forms that use class_name_ref
as the receiver before extracting, so that subsequent suffix IIFE code remains
valid.
Comment on lines
+1160
to
+1187
| .block => { | ||
| const sb = extracted_static_blocks.items[elem.index]; | ||
| const stmts_slice = sb.stmts.slice(); | ||
| rewriteStmts(p, stmts_slice, .{ .replace_this = .{ .ref = class_name_ref, .loc = class_name_loc } }); | ||
|
|
||
| // Check if all statements are simple expressions | ||
| const all_exprs = blk: { | ||
| for (stmts_slice) |sb_stmt| { | ||
| if (sb_stmt.data != .s_expr) break :blk false; | ||
| } | ||
| break :blk true; | ||
| }; | ||
|
|
||
| if (all_exprs) { | ||
| for (stmts_slice) |sb_stmt| { | ||
| suffix_exprs.append(sb_stmt.data.s_expr.value) catch unreachable; | ||
| } | ||
| } else { | ||
| // Wrap in IIFE to preserve non-expression statements | ||
| const iife_body = p.newExpr(E.Arrow{ | ||
| .body = .{ .loc = loc, .stmts = stmts_slice }, | ||
| .is_async = false, | ||
| }, loc); | ||
| suffix_exprs.append(p.newExpr(E.Call{ | ||
| .target = iife_body, | ||
| .args = ExprNodeList.empty, | ||
| }, loc)) catch unreachable; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and locate the relevant code
wc -l src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 87
🏁 Script executed:
# Get context around the lines in question
sed -n '1140,1200p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 3264
🏁 Script executed:
# Search for any handling of `super` in the decorators lowering
rg -n "super" src/ast/lowerDecorators.zig -B 2 -A 2Repository: oven-sh/bun
Length of output: 1030
🏁 Script executed:
# Look for static block extraction patterns
rg -n "static" src/ast/lowerDecorators.zig -B 2 -A 2 | head -100Repository: oven-sh/bun
Length of output: 5583
🏁 Script executed:
# Find the rewriteStmts function definition
rg -n "fn rewriteStmts" src/ast/lowerDecorators.zig -A 30Repository: oven-sh/bun
Length of output: 1815
🏁 Script executed:
# Check if rewriteStmts handles super
rg -n "rewriteStmts" src/ast/lowerDecorators.zig -B 2 -A 5Repository: oven-sh/bun
Length of output: 4235
🏁 Script executed:
# Look for any super references in static block context
rg -n "super" src/ast/lowerDecorators.zig | head -20Repository: oven-sh/bun
Length of output: 358
🏁 Script executed:
# Search the entire codebase for how super is handled in lowering
rg -n "e_super" src/ast/lowerDecorators.zig -B 3 -A 3Repository: oven-sh/bun
Length of output: 515
🏁 Script executed:
# Get the full rewriteExpr function to see all cases
sed -n '195,253p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 2500
🏁 Script executed:
# Check if there's any e_super case in rewriteExpr
rg -n "e_super" src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 161
🏁 Script executed:
# Check the entire RewriteKind enum definition
rg -n "RewriteKind" src/ast/lowerDecorators.zig -B 5 -A 5Repository: oven-sh/bun
Length of output: 1956
🏁 Script executed:
# Search for test cases or comments about static blocks and super
rg -n "super" src/ast/lowerDecorators.zig | grep -i "static"Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for any related test files or documentation
find . -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "static.*super\|super.*static" 2>/dev/null | head -5Repository: oven-sh/bun
Length of output: 91
🏁 Script executed:
# Check what ECMAScript/TC39 specs say about static blocks - search for context
rg -n "static.*block" src/ast/lowerDecorators.zig -i -B 2 -A 2 | head -50Repository: oven-sh/bun
Length of output: 2679
🏁 Script executed:
# Let's look at how static blocks are parsed initially to understand constraints
rg -n "class_static_block" src/ -l | head -5Repository: oven-sh/bun
Length of output: 155
🌐 Web query:
JavaScript static block super semantics ES2022 class static initialization
💡 Result:
In ES2022 class static initialization blocks (static { ... }):
-
Execution timing/order: Static field initializers and
static {}blocks run during class evaluation, in textual order within the class body; and a base class’s static initialization runs before a derived class’s static initialization. [1] -
thisvalue: Inside a static block,thisis the class constructor object (e.g.,B), not an instance. [1] -
supermeaning in a static block:- You may use
super.prop/super[expr]to access static properties/methods on the superclass constructor (e.g.,class B extends A { static { console.log(super.fieldA) } }). [1][2] - You cannot call
super()in a static block — it’s a SyntaxError becausesuper()is only valid in a derived class constructor. [1][3]
- You may use
-
Other notable early restrictions: Using
argumentsis a SyntaxError in a class static initialization block. [1]
Sources: MDN static initialization blocks [1], V8 feature note (super.property in static blocks) [2], MDN error for invalid super() usage [3].
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Static_initialization_blocks
[2] https://v8.dev/features/class-static-initializer-blocks
[3] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Bad_super_call
🏁 Script executed:
# Check if there are any tests related to static blocks in the test file
cat ./test/bundler/esbuild/lower.test.ts | head -100Repository: oven-sh/bun
Length of output: 2722
🏁 Script executed:
# Search for any comments or documentation about static block limitations
rg -n "static.*block" src/ast/lowerDecorators.zig -i -A 5 -B 5 | head -80Repository: oven-sh/bun
Length of output: 4098
🏁 Script executed:
# Check if there's any super detection or special handling before wrapping in IIFE
sed -n '1155,1190p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 2015
🏁 Script executed:
# Search for any Reflect or alternative super handling
rg -n "Reflect" src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check if there's any existing code that detects super in statements
rg -n "\.e_super" src/ -l | head -10Repository: oven-sh/bun
Length of output: 191
🏁 Script executed:
# Look for any visitor or analyzer that walks the AST looking for super
rg -n "containsSuper\|hasSuperAccess\|detectSuper" src/ -iRepository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check where extracted_static_blocks gets populated - trace back from line 1160
sed -n '1000,1165p' src/ast/lowerDecorators.zig | grep -A 5 -B 5 "extracted_static_blocks"Repository: oven-sh/bun
Length of output: 1604
🏁 Script executed:
# Find where original static blocks from source code are handled
rg -n "\.class_static_block" src/ast/lowerDecorators.zig -B 3 -A 3Repository: oven-sh/bun
Length of output: 2673
🏁 Script executed:
# Check if original static blocks are being skipped or included
rg -n "static_element_order\|StaticElement" src/ast/lowerDecorators.zig -B 2 -A 2 | head -80Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for where static blocks from the source class are collected
sed -n '700,900p' src/ast/lowerDecorators.zig | head -150Repository: oven-sh/bun
Length of output: 9581
🏁 Script executed:
# Check if there's any detection of super before wrapping in IIFE
sed -n '1155,1195p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 2301
🏁 Script executed:
# Search for any test cases with static blocks containing super
rg -n "static.*super\|super.*static" test/ -i -A 3 -B 3Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check if there's documentation about known limitations
rg -n "TODO\|FIXME\|XXX.*super\|super.*TODO" src/ast/lowerDecorators.zig -iRepository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's verify: when you extract a static block with super.property to an IIFE, what error would occur?
# Create a test case to understand the issue
cat > /tmp/test_super.js << 'EOF'
// Original (valid)
class B {
static foo = 1;
}
class A extends B {
static {
console.log(super.foo);
}
}
// After extraction to IIFE (invalid)
class A2 extends B {
}
(() => {
console.log(super.foo); // ReferenceError: 'super' keyword is unexpected here
})();
EOF
cat /tmp/test_super.jsRepository: oven-sh/bun
Length of output: 329
Static block extraction breaks super semantics.
When a static block contains super (e.g., class C extends B { static { super.foo(); } }), moving it to a suffix IIFE makes super invalid outside the class body. The code currently extracts original static blocks and wraps non-expression statements in arrow IIFE without detecting or handling super references. Either preserve static blocks containing super in-class or rewrite super accesses (e.g., via Reflect.get/Reflect.set with class_name_ref as receiver) before extraction.
🤖 Prompt for AI Agents
In `@src/ast/lowerDecorators.zig` around lines 1160 - 1187, The extracted
static-block handling currently always moves blocks to suffixes (wrapping
non-expression statements in an Arrow IIFE) which breaks `super` semantics;
update the logic that processes extracted_static_blocks.items[elem.index] (used
with rewriteStmts, suffix_exprs, p.newExpr, E.Arrow, E.Call and
class_name_ref/class_name_loc) to first scan sb.stmts for any Super nodes; if
any `super` reference is present, do not extract/move the block — instead
preserve the original static block inside the class body; alternatively (if you
prefer rewriting), implement a rewrite in rewriteStmts that transforms `super`
property gets/sets/calls into equivalent Reflect.get/Reflect.set or explicit
function call forms that use class_name_ref as the receiver before extracting,
so that subsequent suffix IIFE code remains valid.
Comment on lines
+1313
to
+1335
| // Inject into constructor | ||
| if (constructor_inject_stmts.items.len > 0) { | ||
| var found_constructor = false; | ||
| for (new_properties.items) |*nprop| { | ||
| if (nprop.flags.contains(.is_method) and nprop.key != null and | ||
| nprop.key.?.data == .e_string and nprop.key.?.data.e_string.eqlComptime("constructor")) | ||
| { | ||
| const func = nprop.value.?.data.e_function; | ||
| var body_stmts = ListManaged(Stmt).fromOwnedSlice(p.allocator, func.func.body.stmts); | ||
| var super_index: ?usize = null; | ||
| for (body_stmts.items, 0..) |item, index| { | ||
| if (item.data != .s_expr) continue; | ||
| if (item.data.s_expr.value.data != .e_call) continue; | ||
| if (item.data.s_expr.value.data.e_call.target.data != .e_super) continue; | ||
| super_index = index; | ||
| break; | ||
| } | ||
| const insert_at = if (super_index) |j| j + 1 else 0; | ||
| body_stmts.insertSlice(insert_at, constructor_inject_stmts.items) catch unreachable; | ||
| func.func.body.stmts = body_stmts.items; | ||
| found_constructor = true; | ||
| break; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and the specific section mentioned
head -20 src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 798
🏁 Script executed:
# Get line count to understand file size
wc -l src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 87
🏁 Script executed:
# Look at the specific lines mentioned in the review (1313-1335) plus surrounding context
sed -n '1300,1350p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 2933
🏁 Script executed:
# Search for test files related to decorators and constructor injection
find . -path ./node_modules -prune -o -name "*.test.*" -type f | xargs grep -l "decorator\|super" 2>/dev/null | head -20Repository: oven-sh/bun
Length of output: 1007
🏁 Script executed:
# Search for TypeScript/JavaScript test files related to decorators
find . -path ./node_modules -prune -o \( -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.test.js" \) -type f | xargs grep -l "decorator.*constructor\|@.*decorator" 2>/dev/null | head -10Repository: oven-sh/bun
Length of output: 181
🏁 Script executed:
# Look for any comments or issues mentioning nested super or constructor injection
rg "super\(\)" src/ast/lowerDecorators.zig -B 3 -A 3Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check what statement types exist to understand if super() can be nested
ast-grep --pattern 'enum.*{
$$$
s_$_
$$$
}'Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look at ES decorators test file for constructor super scenarios
grep -n "super" test/bundler/transpiler/es-decorators.test.ts | head -30Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check decorator metadata tests
grep -n "constructor" test/bundler/transpiler/decorator-metadata.test.ts | head -20Repository: oven-sh/bun
Length of output: 284
🏁 Script executed:
# Let's examine more context around the super() insertion logic
sed -n '1280,1370p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 5035
🏁 Script executed:
# Search for any documentation or comments about this logic
grep -n "super" src/ast/lowerDecorators.zig | head -20Repository: oven-sh/bun
Length of output: 358
🏁 Script executed:
# Look at statement types in the JS AST to understand what can contain nested expressions
rg "\.s_" src/ast/lowerDecorators.zig | head -30Repository: oven-sh/bun
Length of output: 1592
🏁 Script executed:
# Check if there are tests with nested super() in constructors
grep -r "super.*(" test/bundler/transpiler/es-decorators.test.ts -A 5 -B 5 | head -100Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for any issue or todo comments related to this
grep -n "nested\|control flow\|if.*super\|try.*super" src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 243
🏁 Script executed:
# Let's check if there's an existing test that exercises super() in nested context
grep -rn "constructor.*{" test/bundler/transpiler/es-decorators.test.ts | grep -A 20 "if\|while\|try\|switch"Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check the actual statement types that can be nested
sed -n '1,100p' src/ast/Stmt.zig | grep -A 50 "pub const Tag"Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search for how super() calls might be handled in control flow
rg "s_if|s_block|s_try" src/ast/lowerDecorators.zig -B 2 -A 2 | head -40Repository: oven-sh/bun
Length of output: 1671
🏁 Script executed:
# Check ECMAScript spec or documentation about super() placement requirements
# Let's look for any parser validation or checker
rg "super" src/ast/ -l | grep -i "check\|valid\|error"Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for how decorators are tested with derived classes
grep -A 30 "extends" test/bundler/transpiler/es-decorators.test.ts | grep -A 20 "constructor"Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's see what the injected statements actually contain
sed -n '1280,1310p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 1691
🏁 Script executed:
# Check if there's validation that super() must be called unconditionally
rg "super.*must\|must.*super" src/ -iRepository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for JS spec or comments about super() placement rules
find . -name "*.md" -o -name "*.txt" | xargs grep -l "super()" 2>/dev/null | head -5Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's verify what the actual injected statements do and confirm they use 'this'
sed -n '1268,1312p' src/ast/lowerDecorators.zig | grep -E "E\.This|newExpr|__runInitializers|__privateAdd"Repository: oven-sh/bun
Length of output: 770
🏁 Script executed:
# Check if there's any parser-level restriction on super() placement
rg "super\(\)" src/parser/ -B 5 -A 5 | head -50Repository: oven-sh/bun
Length of output: 106
🏁 Script executed:
# Let's look at test cases to see if any have super() in conditional/nested context
grep -r "constructor\|super" test/bundler/transpiler/es-decorators.test.ts | head -40Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check JavaScript/TypeScript spec comments or documentation
rg "super.*must|unconditional|first statement" src/ -i | head -20Repository: oven-sh/bun
Length of output: 2203
🏁 Script executed:
# Let's check if any actual test case tries to use decorators with super() in any form
grep -r "decorator" test/bundler/transpiler/ --include="*.test.ts" | grep -i "extends\|super" | head -20Repository: oven-sh/bun
Length of output: 158
🏁 Script executed:
# Check if there are any known issues or discussions about this
rg "@decorator.*class.*extends|extends.*@decorator" test/bundler/transpiler/ -A 10 -B 5 | head -60Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look at a concrete test case with decorators and derived classes
cat test/bundler/transpiler/es-decorators.test.ts | head -200Repository: oven-sh/bun
Length of output: 6128
🏁 Script executed:
# Verify the rule: can JavaScript actually allow super() in nested statements?
# Let's check what the parser validates
find src -name "*.zig" -o -name "*.ts" | xargs grep -l "must call super\|super.*first\|before.*this" 2>/dev/null | head -5Repository: oven-sh/bun
Length of output: 192
🏁 Script executed:
# Find and look at the specific test about extends
grep -n "decorator on class with extends" test/bundler/transpiler/es-decorators.test.ts -A 30Repository: oven-sh/bun
Length of output: 1122
🏁 Script executed:
# Search for super() usage in that test file
grep -n "super" test/bundler/transpiler/es-decorators.test.ts -B 3 -A 3Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's check ES spec rules: in JavaScript, can super() be in conditional branches?
# Looking at actual test patterns
grep -A 50 "extends" test/bundler/transpiler/es-decorators.test.ts | grep -B 5 -A 10 "constructor"Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check if TypeScript/JavaScript validator would even allow super() in nested context
# Let's see if there are any notes about this in the codebase
rg "super.*branch\|branch.*super\|super.*if\|if.*super" src/ -iRepository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's understand the actual ES spec constraint: can super() be in conditional?
# Let's check if there are any comments about this in the implementation
sed -n '1310,1340p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 1700
🏁 Script executed:
# Create a test to see if nested super() is possible and what happens
# Let's check if the test file has any constructor examples
grep -n "constructor" test/bundler/transpiler/es-decorators.test.ts | head -20Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's look at actual test cases with constructors and decorators
grep -B 5 -A 15 "constructor.*{" test/bundler/transpiler/es-decorators.test.ts | head -80Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check if JavaScript even allows super() in nested context
# Let's search for error messages or validation around super
rg "super.*constructor\|constructor.*super" src/ -i | grep -v ".snap\|test" | head -20Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's examine what happens if super() is not found - is this considered an error?
sed -n '1338,1365p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 1727
🏁 Script executed:
# Let me verify the actual JavaScript/TypeScript language rules about super()
# Check if there's any parser validation in the codebase about super() placement
rg "super" src/ -t zig | grep -i "check\|valid\|parse\|error" | head -20Repository: oven-sh/bun
Length of output: 1909
🏁 Script executed:
# Let's look for error handling when super() is in wrong place
rg "super" src/ -t zig -B 2 -A 2 | grep -i "error\|invalid\|bad\|wrong\|must" | head -20Repository: oven-sh/bun
Length of output: 975
🏁 Script executed:
# Check JavaScript spec: super() can only be top-level in constructor
# Let's verify by looking at how parser handles super
fd "parser\|parse" src -type f -name "*.zig" | head -10Repository: oven-sh/bun
Length of output: 225
🏁 Script executed:
# The key insight: In JavaScript/TypeScript, can super() actually appear in nested statements?
# Let's create a simple test to verify the language constraint
cat > /tmp/test_super.js << 'EOF'
class Base {
constructor() {
console.log("base");
}
}
class Derived extends Base {
constructor() {
if (true) {
super(); // Is this valid JavaScript?
}
}
}
new Derived();
EOF
node /tmp/test_super.js 2>&1 || echo "Script failed as expected"Repository: oven-sh/bun
Length of output: 59
🏁 Script executed:
# The test succeeded! super() can be in nested context. Let's verify the actual concern more carefully
# Can we use 'this' before a nested super()?
cat > /tmp/test_this_before_super.js << 'EOF'
class Base {
constructor() {
console.log("base");
}
}
class Derived extends Base {
x = 1;
constructor() {
console.log(this.x); // Use this before nested super
if (true) {
super();
}
}
}
new Derived();
EOF
node /tmp/test_this_before_super.js 2>&1Repository: oven-sh/bun
Length of output: 940
🏁 Script executed:
# Now let's understand the actual issue: if super() is in a nested statement,
# can initializers be inserted before it safely?
# Let me check if the review is about this specific scenario
cat > /tmp/test_init_before_nested_super.js << 'EOF'
class Base {
constructor() {
console.log("base ctor");
}
}
class Derived extends Base {
constructor() {
// Simulate injected initializer that uses this
console.log("init before super");
if (true) {
super();
}
console.log("after super");
}
}
new Derived();
EOF
node /tmp/test_init_before_nested_super.js 2>&1Repository: oven-sh/bun
Length of output: 94
🏁 Script executed:
# Let's look at the full context of the injection to understand the actual use case
sed -n '1200,1250p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 2873
🏁 Script executed:
# Check if there's any special handling for when super is in a derived class
sed -n '1260,1280p' src/ast/lowerDecorators.zigRepository: oven-sh/bun
Length of output: 1134
🏁 Script executed:
# Now I understand the issue. Let me verify if the injected statements indeed use 'this'
grep -n "E.This" src/ast/lowerDecorators.zig | grep "constructor_inject"Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's count how many places use 'this' in constructor injection
sed -n '1268,1335p' src/ast/lowerDecorators.zig | grep -n "E.This"Repository: oven-sh/bun
Length of output: 349
🏁 Script executed:
# Verify: if super() is nested and injections are inserted at position 0,
# they would use 'this' before super() - this causes ReferenceError
# Let's check if there are test cases with nested super() in constructors
rg "constructor.*{" test/bundler/transpiler/es-decorators.test.ts -A 10 | grep -B 5 -A 5 "super\|if\|while\|for"Repository: oven-sh/bun
Length of output: 146
🏁 Script executed:
# Check: is this a realistic scenario? Can super() actually be in nested statements?
# Let me verify if bun's parser would even allow nested super()
rg "allow_super_call" src/ast/parseProperty.zig -B 3 -A 3Repository: oven-sh/bun
Length of output: 572
🏁 Script executed:
# The critical question: in the context of decorators, would super() ever be nested?
# Let's check what constraints exist on super() in constructors
grep -n "is_constructor" src/ast/parseProperty.zig | head -10Repository: oven-sh/bun
Length of output: 405
🏁 Script executed:
# Let's verify the actual issue with a concrete example
# Can we craft a scenario that would trigger this bug?
cat > /tmp/test_decorator_nested_super.js << 'EOF'
function dec(cls, ctx) {
ctx.addInitializer(function() {
console.log("initializer", this.x);
});
return cls;
}
class Base {
constructor() {
console.log("base ctor");
}
}
`@dec` class Derived extends Base {
x = 42;
constructor() {
if (true) {
super();
}
console.log("derived ctor");
}
}
new Derived();
EOF
# Note: can't run this directly without bun, but we understand the structureRepository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Let's verify the code path one more time - what if no super() is found?
sed -n '1313,1340p' src/ast/lowerDecorators.zig | cat -nRepository: oven-sh/bun
Length of output: 1863
🏁 Script executed:
# The key insight: if super_index is null, insert_at = 0
# This means the injected statements go at the beginning of the constructor
# Let's confirm the injected statements use 'this' and would fail before super()
sed -n '1287,1312p' src/ast/lowerDecorators.zig | grep -E "This|__runInitializers|__privateAdd" | head -10Repository: oven-sh/bun
Length of output: 434
Constructor injection fails when super() is in nested control flow.
The scan only detects top-level super() calls. If super() appears in an if/while/for/switch/try/block statement, it won't be found, and decorator initializers (which use this) will be injected at position 0, running before super() and throwing "Must call super constructor in derived class before accessing 'this'" at runtime.
Consider recursively searching for super() in all nested statements, or rewrite each super() call site to inject immediately after it on all code paths.
🤖 Prompt for AI Agents
In `@src/ast/lowerDecorators.zig` around lines 1313 - 1335, The current
constructor injection only looks for top-level super() by scanning
func.func.body.stmts, so when super() is nested in control-flow the code falls
back to inserting at 0; add a recursive search that returns the index of the
enclosing top-level statement that contains a super() call (so we insert after
that statement) — implement a helper (e.g.
findEnclosingStmtIndexWithSuper(stmts: []Stmt) -> ?usize) that iterates
statements: if a statement is an expression-call-to-super return its index,
otherwise for composite statements (s_if, s_block, s_while, s_for, s_switch,
s_try, etc.) recurse into their inner stmt lists and if recursion finds super
return the outer statement's index; replace the existing top-level loop that
computes super_index with a call to this helper and keep the existing insert_at
= if (super_index) |j| j + 1 else 0 and insertion via
body_stmts.insertSlice(...) so decorator initializers are injected after the
correct enclosing statement.
1 task
structwafel
pushed a commit
to structwafel/bun
that referenced
this pull request
Apr 25, 2026
…n-sh#26436) ## Summary - Implement complete lowering of TC39 stage-3 standard ES decorators (the non-legacy variant used when tsconfig has no `experimentalDecorators`). - Passes all 147 esbuild decorator tests and 22 additional Bun-specific tests (191 total, 0 failures). - Supports method, getter, setter, field, auto-accessor, private member, and class decorators in both statement and expression positions, with proper evaluation order, class binding semantics, and decorator metadata. Fixes oven-sh#4122 Fixes oven-sh#20206 Fixes oven-sh#14529 Fixes oven-sh#6051 ## What's implemented | Feature | Details | |---|---| | Method/getter/setter decorators | Static and instance, public and private | | Field decorators | Initializer replacement + extra initializers via `__runInitializers` | | Auto-accessor (`accessor` keyword) | Lowered to WeakMap storage + getter/setter pair | | Private member decorators | WeakMap/WeakSet lowering with `__privateGet`/`__privateSet` | | Class decorators | Statement and expression positions | | Class expression decorators | Comma-expression lowering (no IIFE) | | Decorator metadata | `Symbol.metadata` support via `__decoratorMetadata` | | Evaluation order | All decorator expressions + computed keys evaluated in source order per TC39 spec | | Class binding semantics | Separate inner/outer class name bindings (element vs class decorator closures) | | Static block extraction | `this` replaced with class name ref when moved to suffix | | Computed property keys | Pre-evaluated into temp variables for correct ordering | ## Runtime helpers Added to `src/runtime.js` and registered in `src/runtime.zig`: - `__decoratorStart(base)` — creates decorator context array - `__decorateElement(array, flags, name, decorators, target, extra)` — applies decorators to a class element - `__decoratorMetadata(array, target)` — sets `Symbol.metadata` on the class - `__runInitializers(array, flags, self, value)` — runs initializer/extra-initializer arrays ## Test plan - [x] `bun bd test test/bundler/transpiler/es-decorators-esbuild.test.ts` — **147/147 pass** (esbuild's full decorator test suite) - [x] `bun bd test test/bundler/transpiler/es-decorators.test.ts` — **22/22 pass** - [x] `bun bd test test/bundler/transpiler/decorators.test.ts` — **22/22 pass** (legacy decorators still work) - [x] E2E runtime verification of method, field, accessor, class, private, and expression decorators 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
xhjkl
pushed a commit
to xhjkl/bun
that referenced
this pull request
May 14, 2026
…n-sh#26436) ## Summary - Implement complete lowering of TC39 stage-3 standard ES decorators (the non-legacy variant used when tsconfig has no `experimentalDecorators`). - Passes all 147 esbuild decorator tests and 22 additional Bun-specific tests (191 total, 0 failures). - Supports method, getter, setter, field, auto-accessor, private member, and class decorators in both statement and expression positions, with proper evaluation order, class binding semantics, and decorator metadata. Fixes oven-sh#4122 Fixes oven-sh#20206 Fixes oven-sh#14529 Fixes oven-sh#6051 ## What's implemented | Feature | Details | |---|---| | Method/getter/setter decorators | Static and instance, public and private | | Field decorators | Initializer replacement + extra initializers via `__runInitializers` | | Auto-accessor (`accessor` keyword) | Lowered to WeakMap storage + getter/setter pair | | Private member decorators | WeakMap/WeakSet lowering with `__privateGet`/`__privateSet` | | Class decorators | Statement and expression positions | | Class expression decorators | Comma-expression lowering (no IIFE) | | Decorator metadata | `Symbol.metadata` support via `__decoratorMetadata` | | Evaluation order | All decorator expressions + computed keys evaluated in source order per TC39 spec | | Class binding semantics | Separate inner/outer class name bindings (element vs class decorator closures) | | Static block extraction | `this` replaced with class name ref when moved to suffix | | Computed property keys | Pre-evaluated into temp variables for correct ordering | ## Runtime helpers Added to `src/runtime.js` and registered in `src/runtime.zig`: - `__decoratorStart(base)` — creates decorator context array - `__decorateElement(array, flags, name, decorators, target, extra)` — applies decorators to a class element - `__decoratorMetadata(array, target)` — sets `Symbol.metadata` on the class - `__runInitializers(array, flags, self, value)` — runs initializer/extra-initializer arrays ## Test plan - [x] `bun bd test test/bundler/transpiler/es-decorators-esbuild.test.ts` — **147/147 pass** (esbuild's full decorator test suite) - [x] `bun bd test test/bundler/transpiler/es-decorators.test.ts` — **22/22 pass** - [x] `bun bd test test/bundler/transpiler/decorators.test.ts` — **22/22 pass** (legacy decorators still work) - [x] E2E runtime verification of method, field, accessor, class, private, and expression decorators 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
experimentalDecorators).Fixes #4122
Fixes #20206
Fixes #14529
Fixes #6051
What's implemented
__runInitializersaccessorkeyword)__privateGet/__privateSetSymbol.metadatasupport via__decoratorMetadatathisreplaced with class name ref when moved to suffixRuntime helpers
Added to
src/runtime.jsand registered insrc/runtime.zig:__decoratorStart(base)— creates decorator context array__decorateElement(array, flags, name, decorators, target, extra)— applies decorators to a class element__decoratorMetadata(array, target)— setsSymbol.metadataon the class__runInitializers(array, flags, self, value)— runs initializer/extra-initializer arraysTest plan
bun bd test test/bundler/transpiler/es-decorators-esbuild.test.ts— 147/147 pass (esbuild's full decorator test suite)bun bd test test/bundler/transpiler/es-decorators.test.ts— 22/22 passbun bd test test/bundler/transpiler/decorators.test.ts— 22/22 pass (legacy decorators still work)🤖 Generated with Claude Code