Skip to content

Commit

Permalink
lower decorators for useDefineForClassFields #3913
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 22, 2024
1 parent 46fdb68 commit 9c26f98
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 32 deletions.
58 changes: 58 additions & 0 deletions internal/bundler_tests/bundler_tsconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2869,3 +2869,61 @@ func TestTsconfigJsonExtendsArrayIssue3898(t *testing.T) {
},
})
}

func TestTsconfigDecoratorsUseDefineForClassFieldsFalse(t *testing.T) {
tsconfig_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.ts": `
class Class {
}
class ClassMethod {
foo() {}
}
class ClassField {
foo = 123
bar
}
class ClassAccessor {
accessor foo = 123
accessor bar
}
new Class
new ClassMethod
new ClassField
new ClassAccessor
`,
"/Users/user/project/src/entrywithdec.ts": `
@dec class Class {
}
class ClassMethod {
@dec foo() {}
}
class ClassField {
@dec foo = 123
@dec bar
}
class ClassAccessor {
@dec accessor foo = 123
@dec accessor bar
}
new Class
new ClassMethod
new ClassField
new ClassAccessor
`,
"/Users/user/project/src/tsconfig.json": `{
"compilerOptions": {
"useDefineForClassFields": false
}
}`,
},
entryPaths: []string{
"/Users/user/project/src/entry.ts",
"/Users/user/project/src/entrywithdec.ts",
},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputDir: "/Users/user/project/out",
},
})
}
75 changes: 75 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_tsconfig.txt
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,81 @@ var foo = 123;
// Users/user/project/src/entry.ts
console.log(foo);

================================================================================
TestTsconfigDecoratorsUseDefineForClassFieldsFalse
---------- /Users/user/project/out/entry.js ----------
// Users/user/project/src/entry.ts
var Class = class {
};
var ClassMethod = class {
foo() {
}
};
var ClassField = class {
constructor() {
this.foo = 123;
}
};
var ClassAccessor = class {
accessor foo = 123;
accessor bar;
};
new Class();
new ClassMethod();
new ClassField();
new ClassAccessor();

---------- /Users/user/project/out/entrywithdec.js ----------
// Users/user/project/src/entrywithdec.ts
var _Class_decorators, _init;
_Class_decorators = [dec];
var Class = class {
};
_init = __decoratorStart(null);
Class = __decorateElement(_init, 0, "Class", _Class_decorators, Class);
__runInitializers(_init, 1, Class);
var _foo_dec, _init2;
_foo_dec = [dec];
var ClassMethod = class {
constructor() {
__runInitializers(_init2, 5, this);
}
foo() {
}
};
_init2 = __decoratorStart(null);
__decorateElement(_init2, 1, "foo", _foo_dec, ClassMethod);
__decoratorMetadata(_init2, ClassMethod);
var _bar_dec, _foo_dec2, _init3;
_foo_dec2 = [dec], _bar_dec = [dec];
var ClassField = class {
constructor() {
this.foo = __runInitializers(_init3, 8, this, 123), __runInitializers(_init3, 11, this);
}
};
_init3 = __decoratorStart(null);
__decorateElement(_init3, 5, "foo", _foo_dec2, ClassField);
__decorateElement(_init3, 5, "bar", _bar_dec, ClassField);
__decoratorMetadata(_init3, ClassField);
var _bar_dec2, _foo_dec3, _init4, _foo, _bar;
_foo_dec3 = [dec], _bar_dec2 = [dec];
var ClassAccessor = class {
constructor() {
__privateAdd(this, _foo, __runInitializers(_init4, 8, this, 123)), __runInitializers(_init4, 11, this);
__privateAdd(this, _bar, __runInitializers(_init4, 12, this)), __runInitializers(_init4, 15, this);
}
};
_init4 = __decoratorStart(null);
_foo = new WeakMap();
_bar = new WeakMap();
__decorateElement(_init4, 4, "foo", _foo_dec3, ClassAccessor, _foo);
__decorateElement(_init4, 4, "bar", _bar_dec2, ClassAccessor, _bar);
__decoratorMetadata(_init4, ClassAccessor);
new Class();
new ClassMethod();
new ClassField();
new ClassAccessor();

================================================================================
TestTsconfigExtendsArray
---------- /Users/user/project/out/main.js ----------
Expand Down
9 changes: 9 additions & 0 deletions internal/js_ast/js_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,15 @@ type Class struct {
BodyLoc logger.Loc
CloseBraceLoc logger.Loc

// If true, JavaScript decorators (i.e. not TypeScript experimental
// decorators) should be lowered. This is the case either if JavaScript
// decorators are not supported in the configured target environment, or
// if "useDefineForClassFields" is set to false and this class has
// decorators on it. Note that this flag is not necessarily set to true if
// "useDefineForClassFields" is false and a class has an "accessor" even
// though the accessor feature comes from the decorator specification.
ShouldLowerStandardDecorators bool

// If true, property field initializers cannot be assumed to have no side
// effects. For example:
//
Expand Down
61 changes: 36 additions & 25 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6353,6 +6353,33 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp

closeBraceLoc := p.saveExprCommentsHere()
p.lexer.Expect(js_lexer.TCloseBrace)

// TypeScript has legacy behavior that uses assignment semantics instead of
// define semantics for class fields when "useDefineForClassFields" is enabled
// (in which case TypeScript behaves differently than JavaScript, which is
// arguably "wrong").
//
// This legacy behavior exists because TypeScript added class fields to
// TypeScript before they were added to JavaScript. They decided to go with
// assignment semantics for whatever reason. Later on TC39 decided to go with
// define semantics for class fields instead. This behaves differently if the
// base class has a setter with the same name.
//
// The value of "useDefineForClassFields" defaults to false when it's not
// specified and the target is earlier than "ES2022" since the class field
// language feature was added in ES2022. However, TypeScript's "target"
// setting currently defaults to "ES3" which unfortunately means that the
// "useDefineForClassFields" setting defaults to false (i.e. to "wrong").
//
// We default "useDefineForClassFields" to true (i.e. to "correct") instead.
// This is partially because our target defaults to "esnext", and partially
// because this is a legacy behavior that no one should be using anymore.
// Users that want the wrong behavior can either set "useDefineForClassFields"
// to false in "tsconfig.json" explicitly, or set TypeScript's "target" to
// "ES2021" or earlier in their in "tsconfig.json" file.
useDefineForClassFields := !p.options.ts.Parse || p.options.ts.Config.UseDefineForClassFields == config.True ||
(p.options.ts.Config.UseDefineForClassFields == config.Unspecified && p.options.ts.Config.Target != config.TSTargetBelowES2022)

return js_ast.Class{
ClassKeyword: classKeyword,
Decorators: classOpts.decorators,
Expand All @@ -6362,31 +6389,15 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp
Properties: properties,
CloseBraceLoc: closeBraceLoc,

// TypeScript has legacy behavior that uses assignment semantics instead of
// define semantics for class fields when "useDefineForClassFields" is enabled
// (in which case TypeScript behaves differently than JavaScript, which is
// arguably "wrong").
//
// This legacy behavior exists because TypeScript added class fields to
// TypeScript before they were added to JavaScript. They decided to go with
// assignment semantics for whatever reason. Later on TC39 decided to go with
// define semantics for class fields instead. This behaves differently if the
// base class has a setter with the same name.
//
// The value of "useDefineForClassFields" defaults to false when it's not
// specified and the target is earlier than "ES2022" since the class field
// language feature was added in ES2022. However, TypeScript's "target"
// setting currently defaults to "ES3" which unfortunately means that the
// "useDefineForClassFields" setting defaults to false (i.e. to "wrong").
//
// We default "useDefineForClassFields" to true (i.e. to "correct") instead.
// This is partially because our target defaults to "esnext", and partially
// because this is a legacy behavior that no one should be using anymore.
// Users that want the wrong behavior can either set "useDefineForClassFields"
// to false in "tsconfig.json" explicitly, or set TypeScript's "target" to
// "ES2021" or earlier in their in "tsconfig.json" file.
UseDefineForClassFields: !p.options.ts.Parse || p.options.ts.Config.UseDefineForClassFields == config.True ||
(p.options.ts.Config.UseDefineForClassFields == config.Unspecified && p.options.ts.Config.Target != config.TSTargetBelowES2022),
// Always lower standard decorators if they are present and TypeScript's
// "useDefineForClassFields" setting is false even if the configured target
// environment supports decorators. This setting changes the behavior of
// class fields, and so we must lower decorators so they behave correctly.
ShouldLowerStandardDecorators: (!p.options.ts.Parse && p.options.unsupportedJSFeatures.Has(compat.Decorators)) ||
(p.options.ts.Parse && p.options.ts.Config.ExperimentalDecorators != config.True &&
(p.options.unsupportedJSFeatures.Has(compat.Decorators) || !useDefineForClassFields)),

UseDefineForClassFields: useDefineForClassFields,
}
}

Expand Down
12 changes: 5 additions & 7 deletions internal/js_parser/js_parser_lower_class.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,7 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe
// due to the complexity of the decorator specification. The specification is
// also still evolving so trying to optimize it now is also potentially
// premature.
if p.options.unsupportedJSFeatures.Has(compat.Decorators) &&
(!p.options.ts.Parse || p.options.ts.Config.ExperimentalDecorators != config.True) {
if class.ShouldLowerStandardDecorators {
for _, prop := range class.Properties {
if len(prop.Decorators) > 0 {
for _, prop := range class.Properties {
Expand Down Expand Up @@ -1140,7 +1139,7 @@ func (ctx *lowerClassContext) analyzeProperty(p *parser, prop js_ast.Property, c
// they will end up being lowered (if they are even being lowered at all)
if p.options.ts.Parse && p.options.ts.Config.ExperimentalDecorators == config.True {
analysis.propExperimentalDecorators = prop.Decorators
} else if p.options.unsupportedJSFeatures.Has(compat.Decorators) {
} else if ctx.class.ShouldLowerStandardDecorators {
analysis.propDecorators = prop.Decorators
}

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

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

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

// Evaluate the decorator expressions inline
if p.options.unsupportedJSFeatures.Has(compat.Decorators) && len(ctx.class.Decorators) > 0 &&
(!p.options.ts.Parse || p.options.ts.Config.ExperimentalDecorators != config.True) {
if ctx.class.ShouldLowerStandardDecorators && len(ctx.class.Decorators) > 0 {
name := ctx.nameToKeep
if name == "" {
name = "class"
Expand Down Expand Up @@ -2079,7 +2077,7 @@ func (ctx *lowerClassContext) finishAndGenerateCode(p *parser, result visitClass
if p.options.ts.Parse && p.options.ts.Config.ExperimentalDecorators == config.True {
classExperimentalDecorators = ctx.class.Decorators
ctx.class.Decorators = nil
} else if p.options.unsupportedJSFeatures.Has(compat.Decorators) {
} else if ctx.class.ShouldLowerStandardDecorators {
classDecorators = ctx.decoratorClassDecorators
}

Expand Down

0 comments on commit 9c26f98

Please sign in to comment.