Skip to content

Commit

Permalink
fix #3913: useDefineForClassFields and decorators
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 22, 2024
1 parent 9c26f98 commit 6e049b8
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 4 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
./esbuild --version
```

* Fix class field decorators in TypeScript if `useDefineForClassFields` is `false` ([#3913](https://github.com/evanw/esbuild/issues/3913))

Setting the `useDefineForClassFields` flag to `false` in `tsconfig.json` means class fields use the legacy TypeScript behavior instead of the standard JavaScript behavior. Specifically they use assign semantics instead of define semantics (e.g. setters are triggered) and fields without an initializer are not initialized at all. However, when this legacy behavior is combined with standard JavaScript decorators, TypeScript switches to always initializing all fields, even those without initializers. Previously esbuild incorrectly continued to omit field initializers for this edge case. These field initializers in this case should now be emitted starting with this release.

* Avoid incorrect cycle warning with `tsconfig.json` multiple inheritance ([#3898](https://github.com/evanw/esbuild/issues/3898))

TypeScript 5.0 introduced multiple inheritance for `tsconfig.json` files where `extends` can be an array of file paths. Previously esbuild would incorrectly treat files encountered more than once when processing separate subtrees of the multiple inheritance hierarchy as an inheritance cycle. With this release, `tsconfig.json` files containing this edge case should work correctly without generating a warning.
Expand Down
1 change: 1 addition & 0 deletions internal/bundler_tests/snapshots/snapshots_tsconfig.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ _foo_dec2 = [dec], _bar_dec = [dec];
var ClassField = class {
constructor() {
this.foo = __runInitializers(_init3, 8, this, 123), __runInitializers(_init3, 11, this);
this.bar = __runInitializers(_init3, 12, this), __runInitializers(_init3, 15, this);
}
};
_init3 = __decoratorStart(null);
Expand Down
11 changes: 8 additions & 3 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6287,6 +6287,7 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp
bodyLoc := p.lexer.Loc()
p.lexer.Expect(js_lexer.TOpenBrace)
properties := []js_ast.Property{}
hasPropertyDecorator := false

// Allow "in" and private fields inside class bodies
oldAllowIn := p.allowIn
Expand Down Expand Up @@ -6316,6 +6317,9 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp
firstDecoratorLoc := p.lexer.Loc()
scopeIndex := len(p.scopesInOrder)
opts.decorators = p.parseDecorators(p.currentScope, classKeyword, opts.decoratorContext)
if len(opts.decorators) > 0 {
hasPropertyDecorator = true
}

// This property may turn out to be a type in TypeScript, which should be ignored
if property, ok := p.parseProperty(p.saveExprCommentsHere(), js_ast.PropertyField, opts, nil); ok {
Expand Down Expand Up @@ -6393,9 +6397,10 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp
// "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)),
ShouldLowerStandardDecorators: (len(classOpts.decorators) > 0 || hasPropertyDecorator) &&
((!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
2 changes: 1 addition & 1 deletion internal/js_parser/js_parser_lower_class.go
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ func (ctx *lowerClassContext) analyzeProperty(p *parser, prop js_ast.Property, c
analysis.private, _ = prop.Key.Data.(*js_ast.EPrivateIdentifier)
mustLowerPrivate := analysis.private != nil && p.privateSymbolNeedsToBeLowered(analysis.private)
analysis.shouldOmitFieldInitializer = p.options.ts.Parse && !prop.Kind.IsMethodDefinition() && prop.InitializerOrNil.Data == nil &&
!ctx.class.UseDefineForClassFields && !mustLowerPrivate
!ctx.class.UseDefineForClassFields && !mustLowerPrivate && !ctx.class.ShouldLowerStandardDecorators

// Class fields must be lowered if the environment doesn't support them
if !prop.Kind.IsMethodDefinition() {
Expand Down
28 changes: 28 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -5779,6 +5779,34 @@ for (let flags of [['--target=es2022'], ['--target=es6'], ['--bundle', '--target
}`,
}),

// https://github.com/evanw/esbuild/issues/3913
test(['in.ts', '--outfile=node.js'].concat(flags), {
'in.ts': `
function testDecorator(_value: unknown, context: DecoratorContext) {
if (context.kind === "field") {
return () => "dec-ok";
}
}
class DecClass {
@testDecorator
decInit = "init";
@testDecorator
decNoInit: any;
}
const foo = new DecClass
if (foo.decInit !== 'dec-ok') throw 'fail: decInit'
if (foo.decNoInit !== 'dec-ok') throw 'fail: decNoInit'
`,
'tsconfig.json': `{
"compilerOptions": {
"useDefineForClassFields": false,
},
}`,
}),

// Check various combinations of flags
test(['in.ts', '--outfile=node.js', '--supported:class-field=false'].concat(flags), {
'in.ts': `
Expand Down

0 comments on commit 6e049b8

Please sign in to comment.