fix #3684: abstract experimental decorators

evanw · evanw · commit ae5cc1751655 · 2024-03-14T12:59:49.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,36 @@
 
 ## Unreleased
 
+* Support TypeScript experimental decorators on `abstract` class fields ([#3684](https://github.com/evanw/esbuild/issues/3684))
+
+    With this release, you can now use TypeScript experimental decorators on `abstract` class fields. This was silently compiled incorrectly in esbuild 0.19.7 and below, and was an error from esbuild 0.19.8 to esbuild 0.20.1. Code such as the following should now work correctly:
+
+    ```ts
+    // Original code
+    const log = (x: any, y: string) => console.log(y)
+    abstract class Foo { @log abstract foo: string }
+    new class extends Foo { foo = '' }
+
+    // Old output (with --loader=ts --tsconfig-raw={\"compilerOptions\":{\"experimentalDecorators\":true}})
+    const log = (x, y) => console.log(y);
+    class Foo {
+    }
+    new class extends Foo {
+      foo = "";
+    }();
+
+    // New output (with --loader=ts --tsconfig-raw={\"compilerOptions\":{\"experimentalDecorators\":true}})
+    const log = (x, y) => console.log(y);
+    class Foo {
+    }
+    __decorateClass([
+      log
+    ], Foo.prototype, "foo", 2);
+    new class extends Foo {
+      foo = "";
+    }();
+    ```
+
 * Improve dead code removal of `switch` statements ([#3659](https://github.com/evanw/esbuild/issues/3659))
 
     With this release, esbuild will now remove `switch` statements in branches when minifying if they are known to never be evaluated:
diff --git a/internal/bundler_tests/bundler_ts_test.go b/internal/bundler_tests/bundler_ts_test.go
@@ -1054,6 +1054,7 @@ func TestTSExperimentalDecorators(t *testing.T) {
 					@x @y mDef = 1
 					@x @y method(@x0 @y0 arg0, @x1 @y1 arg1) { return new Foo }
 					@x @y declare mDecl
+					@x @y abstract mAbst
 					constructor(@x0 @y0 arg0, @x1 @y1 arg1) {}
 
 					@x @y static sUndef
@@ -1070,6 +1071,7 @@ func TestTSExperimentalDecorators(t *testing.T) {
 					@x @y [mDef()] = 1
 					@x @y [method()](@x0 @y0 arg0, @x1 @y1 arg1) { return new Foo }
 					@x @y declare [mDecl()]
+					@x @y abstract [mAbst()]
 
 					// Side effect order must be preserved even for fields without decorators
 					[xUndef()]
diff --git a/internal/bundler_tests/snapshots/snapshots_ts.txt b/internal/bundler_tests/snapshots/snapshots_ts.txt
@@ -717,6 +717,10 @@ __decorateClass([
   x,
   y
 ], Foo.prototype, "mDecl", 2);
+__decorateClass([
+  x,
+  y
+], Foo.prototype, "mAbst", 2);
 __decorateClass([
   x,
   y
@@ -747,24 +751,24 @@ Foo = __decorateClass([
 ], Foo);
 
 // all_computed.ts
-var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j;
+var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j, _k;
 var Foo2 = class {
   constructor() {
     this[_b] = 1;
-    this[_e] = 2;
+    this[_f] = 2;
   }
   static {
-    _j = mDecl();
+    _k = mDecl();
   }
   [(_a = mUndef(), _b = mDef(), _c = method())](arg0, arg1) {
     return new Foo2();
   }
-  static [(_d = mDecl(), xUndef(), _e = xDef(), yUndef(), _f = yDef(), _g = sUndef(), _h = sDef(), _i = sMethod())](arg0, arg1) {
+  static [(_d = mDecl(), _e = mAbst(), xUndef(), _f = xDef(), yUndef(), _g = yDef(), _h = sUndef(), _i = sDef(), _j = sMethod())](arg0, arg1) {
     return new Foo2();
   }
 };
-Foo2[_f] = 3;
-Foo2[_h] = new Foo2();
+Foo2[_g] = 3;
+Foo2[_i] = new Foo2();
 __decorateClass([
   x,
   y
@@ -788,23 +792,27 @@ __decorateClass([
 __decorateClass([
   x,
   y
-], Foo2, _g, 2);
+], Foo2.prototype, _e, 2);
 __decorateClass([
   x,
   y
 ], Foo2, _h, 2);
+__decorateClass([
+  x,
+  y
+], Foo2, _i, 2);
 __decorateClass([
   x,
   y,
   __decorateParam(0, x0),
   __decorateParam(0, y0),
   __decorateParam(1, x1),
   __decorateParam(1, y1)
-], Foo2, _i, 1);
+], Foo2, _j, 1);
 __decorateClass([
   x,
   y
-], Foo2, _j, 2);
+], Foo2, _k, 2);
 Foo2 = __decorateClass([
   x?.[_ + "y"](),
   new y?.[_ + "x"]()
diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go
@@ -255,7 +255,7 @@ const (
 	PropertySet
 	PropertyAutoAccessor
 	PropertySpread
-	PropertyDeclare
+	PropertyDeclareOrAbstract
 	PropertyClassStaticBlock
 )
 
diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
@@ -2204,7 +2204,7 @@ func (p *parser) parseProperty(startLoc logger.Loc, kind js_ast.PropertyKind, op
 							//   https://github.com/evanw/esbuild/issues/1675
 							//   https://github.com/microsoft/TypeScript/issues/46345
 							//
-							prop.Kind = js_ast.PropertyDeclare
+							prop.Kind = js_ast.PropertyDeclareOrAbstract
 							return prop, true
 						}
 
@@ -2216,7 +2216,33 @@ func (p *parser) parseProperty(startLoc logger.Loc, kind js_ast.PropertyKind, op
 					if !p.lexer.HasNewlineBefore && opts.isClass && p.options.ts.Parse && !opts.isTSAbstract && raw == name.String {
 						opts.isTSAbstract = true
 						scopeIndex := len(p.scopesInOrder)
-						p.parseProperty(startLoc, kind, opts, nil)
+
+						if prop, ok := p.parseProperty(startLoc, kind, opts, nil); ok &&
+							prop.Kind == js_ast.PropertyNormal && prop.ValueOrNil.Data == nil &&
+							(p.options.ts.Config.ExperimentalDecorators == config.True && len(opts.decorators) > 0) {
+							// If this is a well-formed class field with the "abstract" keyword,
+							// only keep the declaration to preserve its side-effects when
+							// there are TypeScript experimental decorators present:
+							//
+							//   abstract class Foo {
+							//     // Remove this
+							//     abstract [(console.log('side effect 1'), 'foo')]
+							//
+							//     // Keep this
+							//     @decorator(console.log('side effect 2')) abstract bar
+							//   }
+							//
+							// This behavior is valid with TypeScript experimental decorators.
+							// TypeScript does not allow this with JavaScript decorators.
+							//
+							// References:
+							//
+							//   https://github.com/evanw/esbuild/issues/3684
+							//
+							prop.Kind = js_ast.PropertyDeclareOrAbstract
+							return prop, true
+						}
+
 						p.discardScopesUpTo(scopeIndex)
 						return js_ast.Property{}, false
 					}
diff --git a/internal/js_parser/js_parser_lower_class.go b/internal/js_parser/js_parser_lower_class.go
@@ -1062,10 +1062,10 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas
 			}
 		}
 
-		// If the field uses the TypeScript "declare" keyword, just omit it entirely.
-		// However, we must still keep any side-effects in the computed value and/or
-		// in the decorators.
-		if prop.Kind == js_ast.PropertyDeclare && prop.ValueOrNil.Data == nil {
+		// If the field uses the TypeScript "declare" or "abstract" keyword, just
+		// omit it entirely. However, we must still keep any side-effects in the
+		// computed value and/or in the decorators.
+		if prop.Kind == js_ast.PropertyDeclareOrAbstract && prop.ValueOrNil.Data == nil {
 			mustLowerField = true
 			shouldOmitFieldInitializer = true
 		}
diff --git a/internal/js_parser/ts_parser_test.go b/internal/js_parser/ts_parser_test.go
@@ -2051,9 +2051,11 @@ func TestTSExperimentalDecorator(t *testing.T) {
 	expectParseErrorExperimentalDecoratorTS(t, "@x export default abstract", "<stdin>: ERROR: Decorators are not valid here\n")
 	expectParseErrorExperimentalDecoratorTS(t, "@x export @y default class {}", "<stdin>: ERROR: Decorators are not valid here\n<stdin>: ERROR: Unexpected \"default\"\n")
 
-	// TypeScript experimental decorators are actually allowed on declared fields
+	// TypeScript experimental decorators are actually allowed on declared and abstract fields
 	expectPrintedExperimentalDecoratorTS(t, "class Foo { @(() => {}) declare foo: any; @(() => {}) bar: any }",
 		"class Foo {\n  bar;\n}\n__decorateClass([\n  () => {\n  }\n], Foo.prototype, \"foo\", 2);\n__decorateClass([\n  () => {\n  }\n], Foo.prototype, \"bar\", 2);\n")
+	expectPrintedExperimentalDecoratorTS(t, "abstract class Foo { @(() => {}) abstract foo: any; @(() => {}) bar: any }",
+		"class Foo {\n  bar;\n}\n__decorateClass([\n  () => {\n  }\n], Foo.prototype, \"foo\", 2);\n__decorateClass([\n  () => {\n  }\n], Foo.prototype, \"bar\", 2);\n")
 }
 
 func TestTSDecorators(t *testing.T) {
@@ -2130,9 +2132,11 @@ func TestTSDecorators(t *testing.T) {
 	expectParseErrorTS(t, "@x export default abstract", "<stdin>: ERROR: Decorators are not valid here\n")
 	expectParseErrorTS(t, "@x export @y default class {}", "<stdin>: ERROR: Decorators are not valid here\n<stdin>: ERROR: Unexpected \"default\"\n")
 
-	// JavaScript decorators are not allowed on declared fields
+	// JavaScript decorators are not allowed on declared or abstract fields
 	expectParseErrorTS(t, "class Foo { @(() => {}) declare foo: any; @(() => {}) bar: any }",
 		"<stdin>: ERROR: Decorators are not valid here\n")
+	expectParseErrorTS(t, "abstract class Foo { @(() => {}) abstract foo: any; @(() => {}) bar: any }",
+		"<stdin>: ERROR: Decorators are not valid here\n")
 }
 
 func TestTSTry(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -255,7 +255,7 @@ const (`
`255`	`255`	`PropertySet`
`256`	`256`	`PropertyAutoAccessor`
`257`	`257`	`PropertySpread`
`258`		`- PropertyDeclare`
	`258`	`+ PropertyDeclareOrAbstract`
`259`	`259`	`PropertyClassStaticBlock`
`260`	`260`	`)`
`261`	`261`
Original file line number	Diff line number	Diff line change
`@@ -1062,10 +1062,10 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas`
`1062`	`1062`	`}`
`1063`	`1063`	`}`
`1064`	`1064`
`1065`		`- // If the field uses the TypeScript "declare" keyword, just omit it entirely.`
`1066`		`- // However, we must still keep any side-effects in the computed value and/or`
`1067`		`- // in the decorators.`
`1068`		`- if prop.Kind == js_ast.PropertyDeclare && prop.ValueOrNil.Data == nil {`
	`1065`	`+ // If the field uses the TypeScript "declare" or "abstract" keyword, just`
	`1066`	`+ // omit it entirely. However, we must still keep any side-effects in the`
	`1067`	`+ // computed value and/or in the decorators.`
	`1068`	`+ if prop.Kind == js_ast.PropertyDeclareOrAbstract && prop.ValueOrNil.Data == nil {`
`1069`	`1069`	`mustLowerField = true`
`1070`	`1070`	`shouldOmitFieldInitializer = true`
`1071`	`1071`	`}`