Skip to content

Commit 9c26f98

Browse files
evanwevanw
committed
lower decorators for useDefineForClassFields #3913 
1 parent 46fdb68 commit 9c26f98

File tree

5 files changed

+183
-32
lines changed

5 files changed

+183
-32
lines changed

‎internal/bundler_tests/bundler_tsconfig_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2869,3 +2869,61 @@ func TestTsconfigJsonExtendsArrayIssue3898(t *testing.T) {
28692869
},
28702870
})
28712871
}
2872+
2873+
func TestTsconfigDecoratorsUseDefineForClassFieldsFalse(t *testing.T) {
2874+
tsconfig_suite.expectBundled(t, bundled{
2875+
files: map[string]string{
2876+
"/Users/user/project/src/entry.ts": `
2877+
class Class {
2878+
}
2879+
class ClassMethod {
2880+
foo() {}
2881+
}
2882+
class ClassField {
2883+
foo = 123
2884+
bar
2885+
}
2886+
class ClassAccessor {
2887+
accessor foo = 123
2888+
accessor bar
2889+
}
2890+
new Class
2891+
new ClassMethod
2892+
new ClassField
2893+
new ClassAccessor
2894+
`,
2895+
"/Users/user/project/src/entrywithdec.ts": `
2896+
@dec class Class {
2897+
}
2898+
class ClassMethod {
2899+
@dec foo() {}
2900+
}
2901+
class ClassField {
2902+
@dec foo = 123
2903+
@dec bar
2904+
}
2905+
class ClassAccessor {
2906+
@dec accessor foo = 123
2907+
@dec accessor bar
2908+
}
2909+
new Class
2910+
new ClassMethod
2911+
new ClassField
2912+
new ClassAccessor
2913+
`,
2914+
"/Users/user/project/src/tsconfig.json": `{
2915+
"compilerOptions": {
2916+
"useDefineForClassFields": false
2917+
}
2918+
}`,
2919+
},
2920+
entryPaths: []string{
2921+
"/Users/user/project/src/entry.ts",
2922+
"/Users/user/project/src/entrywithdec.ts",
2923+
},
2924+
options: config.Options{
2925+
Mode: config.ModeBundle,
2926+
AbsOutputDir: "/Users/user/project/out",
2927+
},
2928+
})
2929+
}

‎internal/bundler_tests/snapshots/snapshots_tsconfig.txt

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,81 @@ var foo = 123;
8282
// Users/user/project/src/entry.ts
8383
console.log(foo);
8484

85+
================================================================================
86+
TestTsconfigDecoratorsUseDefineForClassFieldsFalse
87+
---------- /Users/user/project/out/entry.js ----------
88+
// Users/user/project/src/entry.ts
89+
var Class = class {
90+
};
91+
var ClassMethod = class {
92+
foo() {
93+
}
94+
};
95+
var ClassField = class {
96+
constructor() {
97+
this.foo = 123;
98+
}
99+
};
100+
var ClassAccessor = class {
101+
accessor foo = 123;
102+
accessor bar;
103+
};
104+
new Class();
105+
new ClassMethod();
106+
new ClassField();
107+
new ClassAccessor();
108+
109+
---------- /Users/user/project/out/entrywithdec.js ----------
110+
// Users/user/project/src/entrywithdec.ts
111+
var _Class_decorators, _init;
112+
_Class_decorators = [dec];
113+
var Class = class {
114+
};
115+
_init = __decoratorStart(null);
116+
Class = __decorateElement(_init, 0, "Class", _Class_decorators, Class);
117+
__runInitializers(_init, 1, Class);
118+
var _foo_dec, _init2;
119+
_foo_dec = [dec];
120+
var ClassMethod = class {
121+
constructor() {
122+
__runInitializers(_init2, 5, this);
123+
}
124+
foo() {
125+
}
126+
};
127+
_init2 = __decoratorStart(null);
128+
__decorateElement(_init2, 1, "foo", _foo_dec, ClassMethod);
129+
__decoratorMetadata(_init2, ClassMethod);
130+
var _bar_dec, _foo_dec2, _init3;
131+
_foo_dec2 = [dec], _bar_dec = [dec];
132+
var ClassField = class {
133+
constructor() {
134+
this.foo = __runInitializers(_init3, 8, this, 123), __runInitializers(_init3, 11, this);
135+
}
136+
};
137+
_init3 = __decoratorStart(null);
138+
__decorateElement(_init3, 5, "foo", _foo_dec2, ClassField);
139+
__decorateElement(_init3, 5, "bar", _bar_dec, ClassField);
140+
__decoratorMetadata(_init3, ClassField);
141+
var _bar_dec2, _foo_dec3, _init4, _foo, _bar;
142+
_foo_dec3 = [dec], _bar_dec2 = [dec];
143+
var ClassAccessor = class {
144+
constructor() {
145+
__privateAdd(this, _foo, __runInitializers(_init4, 8, this, 123)), __runInitializers(_init4, 11, this);
146+
__privateAdd(this, _bar, __runInitializers(_init4, 12, this)), __runInitializers(_init4, 15, this);
147+
}
148+
};
149+
_init4 = __decoratorStart(null);
150+
_foo = new WeakMap();
151+
_bar = new WeakMap();
152+
__decorateElement(_init4, 4, "foo", _foo_dec3, ClassAccessor, _foo);
153+
__decorateElement(_init4, 4, "bar", _bar_dec2, ClassAccessor, _bar);
154+
__decoratorMetadata(_init4, ClassAccessor);
155+
new Class();
156+
new ClassMethod();
157+
new ClassField();
158+
new ClassAccessor();
159+
85160
================================================================================
86161
TestTsconfigExtendsArray
87162
---------- /Users/user/project/out/main.js ----------

‎internal/js_ast/js_ast.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,15 @@ type Class struct {
374374
BodyLoc logger.Loc
375375
CloseBraceLoc logger.Loc
376376

377+
// If true, JavaScript decorators (i.e. not TypeScript experimental
378+
// decorators) should be lowered. This is the case either if JavaScript
379+
// decorators are not supported in the configured target environment, or
380+
// if "useDefineForClassFields" is set to false and this class has
381+
// decorators on it. Note that this flag is not necessarily set to true if
382+
// "useDefineForClassFields" is false and a class has an "accessor" even
383+
// though the accessor feature comes from the decorator specification.
384+
ShouldLowerStandardDecorators bool
385+
377386
// If true, property field initializers cannot be assumed to have no side
378387
// effects. For example:
379388
//

‎internal/js_parser/js_parser.go

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6353,6 +6353,33 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp
63536353

63546354
closeBraceLoc := p.saveExprCommentsHere()
63556355
p.lexer.Expect(js_lexer.TCloseBrace)
6356+
6357+
// TypeScript has legacy behavior that uses assignment semantics instead of
6358+
// define semantics for class fields when "useDefineForClassFields" is enabled
6359+
// (in which case TypeScript behaves differently than JavaScript, which is
6360+
// arguably "wrong").
6361+
//
6362+
// This legacy behavior exists because TypeScript added class fields to
6363+
// TypeScript before they were added to JavaScript. They decided to go with
6364+
// assignment semantics for whatever reason. Later on TC39 decided to go with
6365+
// define semantics for class fields instead. This behaves differently if the
6366+
// base class has a setter with the same name.
6367+
//
6368+
// The value of "useDefineForClassFields" defaults to false when it's not
6369+
// specified and the target is earlier than "ES2022" since the class field
6370+
// language feature was added in ES2022. However, TypeScript's "target"
6371+
// setting currently defaults to "ES3" which unfortunately means that the
6372+
// "useDefineForClassFields" setting defaults to false (i.e. to "wrong").
6373+
//
6374+
// We default "useDefineForClassFields" to true (i.e. to "correct") instead.
6375+
// This is partially because our target defaults to "esnext", and partially
6376+
// because this is a legacy behavior that no one should be using anymore.
6377+
// Users that want the wrong behavior can either set "useDefineForClassFields"
6378+
// to false in "tsconfig.json" explicitly, or set TypeScript's "target" to
6379+
// "ES2021" or earlier in their in "tsconfig.json" file.
6380+
useDefineForClassFields := !p.options.ts.Parse || p.options.ts.Config.UseDefineForClassFields == config.True ||
6381+
(p.options.ts.Config.UseDefineForClassFields == config.Unspecified && p.options.ts.Config.Target != config.TSTargetBelowES2022)
6382+
63566383
return js_ast.Class{
63576384
ClassKeyword: classKeyword,
63586385
Decorators: classOpts.decorators,
@@ -6362,31 +6389,15 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp
63626389
Properties: properties,
63636390
CloseBraceLoc: closeBraceLoc,
63646391

6365-
// TypeScript has legacy behavior that uses assignment semantics instead of
6366-
// define semantics for class fields when "useDefineForClassFields" is enabled
6367-
// (in which case TypeScript behaves differently than JavaScript, which is
6368-
// arguably "wrong").
6369-
//
6370-
// This legacy behavior exists because TypeScript added class fields to
6371-
// TypeScript before they were added to JavaScript. They decided to go with
6372-
// assignment semantics for whatever reason. Later on TC39 decided to go with
6373-
// define semantics for class fields instead. This behaves differently if the
6374-
// base class has a setter with the same name.
6375-
//
6376-
// The value of "useDefineForClassFields" defaults to false when it's not
6377-
// specified and the target is earlier than "ES2022" since the class field
6378-
// language feature was added in ES2022. However, TypeScript's "target"
6379-
// setting currently defaults to "ES3" which unfortunately means that the
6380-
// "useDefineForClassFields" setting defaults to false (i.e. to "wrong").
6381-
//
6382-
// We default "useDefineForClassFields" to true (i.e. to "correct") instead.
6383-
// This is partially because our target defaults to "esnext", and partially
6384-
// because this is a legacy behavior that no one should be using anymore.
6385-
// Users that want the wrong behavior can either set "useDefineForClassFields"
6386-
// to false in "tsconfig.json" explicitly, or set TypeScript's "target" to
6387-
// "ES2021" or earlier in their in "tsconfig.json" file.
6388-
UseDefineForClassFields: !p.options.ts.Parse || p.options.ts.Config.UseDefineForClassFields == config.True ||
6389-
(p.options.ts.Config.UseDefineForClassFields == config.Unspecified && p.options.ts.Config.Target != config.TSTargetBelowES2022),
6392+
// Always lower standard decorators if they are present and TypeScript's
6393+
// "useDefineForClassFields" setting is false even if the configured target
6394+
// environment supports decorators. This setting changes the behavior of
6395+
// class fields, and so we must lower decorators so they behave correctly.
6396+
ShouldLowerStandardDecorators: (!p.options.ts.Parse && p.options.unsupportedJSFeatures.Has(compat.Decorators)) ||
6397+
(p.options.ts.Parse && p.options.ts.Config.ExperimentalDecorators != config.True &&
6398+
(p.options.unsupportedJSFeatures.Has(compat.Decorators) || !useDefineForClassFields)),
6399+
6400+
UseDefineForClassFields: useDefineForClassFields,
63906401
}
63916402
}
63926403

‎internal/js_parser/js_parser_lower_class.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -416,8 +416,7 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe
416416
// due to the complexity of the decorator specification. The specification is
417417
// also still evolving so trying to optimize it now is also potentially
418418
// premature.
419-
if p.options.unsupportedJSFeatures.Has(compat.Decorators) &&
420-
(!p.options.ts.Parse || p.options.ts.Config.ExperimentalDecorators != config.True) {
419+
if class.ShouldLowerStandardDecorators {
421420
for _, prop := range class.Properties {
422421
if len(prop.Decorators) > 0 {
423422
for _, prop := range class.Properties {
@@ -1140,7 +1139,7 @@ func (ctx *lowerClassContext) analyzeProperty(p *parser, prop js_ast.Property, c
11401139
// they will end up being lowered (if they are even being lowered at all)
11411140
if p.options.ts.Parse && p.options.ts.Config.ExperimentalDecorators == config.True {
11421141
analysis.propExperimentalDecorators = prop.Decorators
1143-
} else if p.options.unsupportedJSFeatures.Has(compat.Decorators) {
1142+
} else if ctx.class.ShouldLowerStandardDecorators {
11441143
analysis.propDecorators = prop.Decorators
11451144
}
11461145

@@ -1451,7 +1450,7 @@ func (ctx *lowerClassContext) processProperties(p *parser, classLoweringInfo cla
14511450
propertyKeyTempRefs, decoratorTempRefs := ctx.hoistComputedProperties(p, classLoweringInfo)
14521451

14531452
// Save the initializer index for each field and accessor element
1454-
if p.options.unsupportedJSFeatures.Has(compat.Decorators) && (!p.options.ts.Parse || p.options.ts.Config.ExperimentalDecorators != config.True) {
1453+
if ctx.class.ShouldLowerStandardDecorators {
14551454
var counts [4]int
14561455

14571456
// Count how many initializers there are in each section
@@ -1484,8 +1483,7 @@ func (ctx *lowerClassContext) processProperties(p *parser, classLoweringInfo cla
14841483
}
14851484

14861485
// Evaluate the decorator expressions inline
1487-
if p.options.unsupportedJSFeatures.Has(compat.Decorators) && len(ctx.class.Decorators) > 0 &&
1488-
(!p.options.ts.Parse || p.options.ts.Config.ExperimentalDecorators != config.True) {
1486+
if ctx.class.ShouldLowerStandardDecorators && len(ctx.class.Decorators) > 0 {
14891487
name := ctx.nameToKeep
14901488
if name == "" {
14911489
name = "class"
@@ -2079,7 +2077,7 @@ func (ctx *lowerClassContext) finishAndGenerateCode(p *parser, result visitClass
20792077
if p.options.ts.Parse && p.options.ts.Config.ExperimentalDecorators == config.True {
20802078
classExperimentalDecorators = ctx.class.Decorators
20812079
ctx.class.Decorators = nil
2082-
} else if p.options.unsupportedJSFeatures.Has(compat.Decorators) {
2080+
} else if ctx.class.ShouldLowerStandardDecorators {
20832081
classDecorators = ctx.decoratorClassDecorators
20842082
}
20852083

0 commit comments

Comments
 (0)