From 6e049b81d2e080ccdf24db84a71934ea736879ce Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sat, 21 Sep 2024 21:11:11 -0400 Subject: [PATCH] fix #3913: useDefineForClassFields and decorators --- CHANGELOG.md | 4 +++ .../snapshots/snapshots_tsconfig.txt | 1 + internal/js_parser/js_parser.go | 11 ++++++-- internal/js_parser/js_parser_lower_class.go | 2 +- scripts/end-to-end-tests.js | 28 +++++++++++++++++++ 5 files changed, 42 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2c274aa943..861d8b94ac4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/internal/bundler_tests/snapshots/snapshots_tsconfig.txt b/internal/bundler_tests/snapshots/snapshots_tsconfig.txt index 35f690e8093..a83bf1a74d4 100644 --- a/internal/bundler_tests/snapshots/snapshots_tsconfig.txt +++ b/internal/bundler_tests/snapshots/snapshots_tsconfig.txt @@ -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); diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 7e11b3d6eb9..45daca94c3f 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -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 @@ -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 { @@ -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, } diff --git a/internal/js_parser/js_parser_lower_class.go b/internal/js_parser/js_parser_lower_class.go index 3e7366140a0..01458368cf1 100644 --- a/internal/js_parser/js_parser_lower_class.go +++ b/internal/js_parser/js_parser_lower_class.go @@ -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() { diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 9ae26740f86..89049400d69 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -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': `