fix #3913: useDefineForClassFields and decorators

evanw · evanw · commit 6e049b81d2e0 · 2024-09-21T21:11:11.000-04:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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': `

Original file line number	Diff line number	Diff line change
`@@ -132,6 +132,7 @@ _foo_dec2 = [dec], _bar_dec = [dec];`
`132`	`132`	`var ClassField = class {`
`133`	`133`	`constructor() {`
`134`	`134`	`this.foo = __runInitializers(_init3, 8, this, 123), __runInitializers(_init3, 11, this);`
	`135`	`+ this.bar = __runInitializers(_init3, 12, this), __runInitializers(_init3, 15, this);`
`135`	`136`	`}`
`136`	`137`	`};`
`137`	`138`	`_init3 = __decoratorStart(null);`