fix(parser): allow accessor keyword with experimentalDecorators: true - #27336
fix(parser): allow accessor keyword with experimentalDecorators: true#27336robobun wants to merge 1 commit into
accessor keyword with experimentalDecorators: true#27336Conversation
…rue` The `accessor` keyword (TC39 auto-accessors proposal) was incorrectly gated behind the `standard_decorators` feature flag, causing it to be rejected as a syntax error when `experimentalDecorators: true` was set in tsconfig.json. TypeScript supports `accessor` in both decorator modes. Two changes: - Remove `standard_decorators` guard from accessor keyword recognition in the parser so it's always recognized in class bodies. - Set `should_lower_standard_decorators` when auto-accessors are present regardless of the decorator mode, since auto-accessors always need the standard lowering path (WeakMap + getter/setter transformation). Closes #27335 Co-Authored-By: Claude <noreply@anthropic.com>
|
Updated 10:48 PM PT - Feb 21st, 2026
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 27336That installs a local version of the PR into your bun-27336 --bun |
|
No actionable comments were generated in the recent review. 🎉 WalkthroughSupport for TypeScript's auto-accessor keyword (stage 3 ECMAScript proposal) is now recognized unconditionally within classes. The parser's decorator lowering logic was adjusted, and comprehensive regression tests were added to validate accessor behavior across various decorator configurations. Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
🔴 b79be — 1 issue(s) found
|
| .properties = properties.items, | ||
| .has_decorators = has_any_decorators, | ||
| .should_lower_standard_decorators = p.options.features.standard_decorators and (has_any_decorators or has_auto_accessor), | ||
| .should_lower_standard_decorators = has_auto_accessor or (p.options.features.standard_decorators and has_any_decorators), |
There was a problem hiding this comment.
🔴 Critical: Bug: when experimentalDecorators: true and a class has an accessor field, should_lower_standard_decorators is set to true because has_auto_accessor is now evaluated before the standard_decorators guard. This routes the entire class (including any legacy experimental decorators on other members) through the TC39 standard decorator lowering path (lowerStandardDecoratorsStmt / __decorateElement), which calls decorators with (value, context) instead of the expected experimental signature (target, key, descriptor). The fix should gate has_auto_accessor on standard_decorators as well, or handle accessor lowering separately from the decorator lowering path when in experimental mode.
Why this is a problem
What the Bug Is
The PR changes the assignment of should_lower_standard_decorators from:
p.options.features.standard_decorators and (has_any_decorators or has_auto_accessor)to:
has_auto_accessor or (p.options.features.standard_decorators and has_any_decorators)This boolean logic change means that the presence of an accessor field alone is now sufficient to set should_lower_standard_decorators = true, regardless of whether the file is using standard or experimental decorators. In the old code, standard_decorators was a prerequisite for the entire expression, ensuring experimental decorator mode could never reach the standard lowering path. In the new code, has_auto_accessor short-circuits past that guard.
The Specific Code Path
When experimentalDecorators: true is set in a TypeScript project's tsconfig.json, the transpiler sets opts.features.standard_decorators = false (confirmed at src/transpiler.zig:1106 and src/bundler/ParseTask.zig:1217). Under these conditions, a class like:
class Foo {
accessor x = 1;
@log greet() { return "hello"; }
}will have has_auto_accessor = true (from the accessor x field) and has_any_decorators = true (from the @log decorator). With the new code, should_lower_standard_decorators evaluates to true or (false and true) = true. In P.zig lowerClass (line 4873), when should_lower_standard_decorators is true, the class is immediately routed to lowerStandardDecoratorsStmt which returns, completely bypassing the legacy experimental decorator path starting at line 4884.
Why Existing Tests Don't Catch It
The PR includes a test case (test case 4: "accessor with experimental decorators on other members") that appears to validate this scenario, but the @log decorator used in that test is a no-op: function log(target: any, key: string) {}. Since it discards its arguments, it produces no observable difference regardless of whether it's called with (value, context) (standard TC39 protocol) or (target, key, descriptor) (experimental protocol). A real-world experimental decorator that inspects or modifies target, key, or descriptor would silently receive incorrect arguments.
Step-by-Step Proof
- User has
experimentalDecorators: truein tsconfig.json, sostandard_decorators = false. - User writes a class with both
accessor x = 1and@myDecorator greet() {}, wheremyDecoratoris an experimental-style decorator expecting(target, key, descriptor). - The parser sets
has_auto_accessor = trueandhas_any_decorators = true. - Line 212 evaluates:
should_lower_standard_decorators = true or (false and true) = true. - In
P.ziglowerClass (line 4873), the conditionstmt.data.s_class.class.should_lower_standard_decoratorsistrue. lowerStandardDecoratorsStmtis called, which processes ALL decorators in the class using__decorateElement(inlowerDecorators.zig), calling each decorator with the TC39 standard protocol(value, context).@myDecoratorreceives(value, context)instead of(target, key, descriptor). Thetargetparameter gets a function value instead of the class prototype,keygets a context object instead of the method name string, anddescriptorisundefined.- The decorator silently produces incorrect behavior or throws a runtime error.
How to Fix It
The fix should ensure that has_auto_accessor only triggers the standard decorator lowering path when standard_decorators is also true. One approach is:
.should_lower_standard_decorators = p.options.features.standard_decorators and (has_any_decorators or has_auto_accessor),(i.e., reverting to the original logic) and handling the accessor keyword lowering for experimental decorator mode through a separate mechanism. Alternatively, if the intent is to support accessor with experimental decorators, the accessor field should be lowered independently of the decorator lowering path, perhaps by transforming it into a getter/setter pair without routing through lowerStandardDecoratorsStmt.
|
Superseded by #29201 — same root cause, with a proper |
Summary
accessorkeyword (TC39 auto-accessors proposal) being rejected as a syntax error whenexperimentalDecorators: trueis set intsconfig.jsonaccessorkeyword was incorrectly gated behind thestandard_decoratorsfeature flag in the parser, but TypeScript supports it in both decorator modesCloses #27335
Test plan
test/regression/issue/27335.test.tscovering:accessorwithexperimentalDecorators: trueaccessorwithpublic,private,staticmodifiers +experimentalDecorators: trueaccessorwithoutexperimentalDecorators(standard mode, existing behavior)accessorfields + experimental decorators on other class membersUSE_SYSTEM_BUN=1), pass with debug buildes-decorators.test.ts(27 pass)es-decorators-esbuild.test.ts(147 pass)decorators.test.ts(22 pass)decorator-metadata.test.ts(5 pass)🤖 Generated with Claude Code