fix #1507: wrong ts class field side effect order

CHANGELOG.md

@@ -6,6 +6,41 @@
 
     With this release, esbuild will now generate source maps for CSS output files when `--sourcemap` is enabled. This supports all of the same options as JS source maps including `--sourcemap=inline` and `--sourcemap=external`. In addition, CSS input files with embedded `/*# sourceMappingURL=... */` comments will cause the CSS output file source map to map all the way back to the original inputs. CSS source maps are used by the browser's style inspector to link back to the original source code instead of linking to the bundled source code.
 
+* Fix computed class fields in TypeScript edge case ([#1507](https://github.com/evanw/esbuild/issues/1507))
+
+    If TypeScript code contains computed class fields, the target environment supports class fields so syntax lowering is not necessary, and TypeScript's `useDefineForClassFields` setting is set to `true`, then esbuild had a bug where the computed property names were computed after the class definition and were undefined. Note that TypeScript's `useDefineForClassFields` setting defaults to `true` if `tsconfig.json` contains `"target": "ESNext"`.
+
+    ```ts
+    // Original code
+    class Foo {
+      [foo] = 1;
+      @bar [baz] = 2;
+    }
+
+    // Old output
+    var _a, _b;
+    var Foo = class {
+      [_a] = 1;
+      [_b] = 2;
+    };
+    _a = foo, _b = baz;
+    __decorateClass([
+      bar
+    ], Foo.prototype, _b, 2);
+
+    // New output
+    var _a;
+    var Foo = class {
+      [foo] = 1;
+      [_a = baz] = 2;
+    };
+    __decorateClass([
+      bar
+    ], Foo.prototype, _a, 2);
+    ```
+
+    The problem in this case is that normally TypeScript moves class field initializers into the special `constructor` method (automatically generating one if one doesn't already exist) so the side effects for class field property names must happen after the class body. But if class fields are supported by the target environment then the side effects must happen inline instead.
+
 ## 0.12.18
 
 * Allow implicit `./` in CSS `@import` paths ([#1494](https://github.com/evanw/esbuild/pull/1494))

internal/bundler/bundler_ts_test.go

@@ -1121,3 +1121,76 @@ func TestThisInsideFunctionTSNoBundleUseDefineForClassFields(t *testing.T) {
 		},
 	})
 }
+
+func TestTSComputedClassFieldUseDefineFalse(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.ts": `
+				class Foo {
+					[q];
+					[r] = s;
+					@dec
+					[x];
+					@dec
+					[y] = z;
+				}
+				new Foo()
+			`,
+		},
+		entryPaths: []string{"/entry.ts"},
+		options: config.Options{
+			Mode:                    config.ModePassThrough,
+			AbsOutputFile:           "/out.js",
+			UseDefineForClassFields: config.False,
+		},
+	})
+}
+
+func TestTSComputedClassFieldUseDefineTrue(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.ts": `
+				class Foo {
+					[q];
+					[r] = s;
+					@dec
+					[x];
+					@dec
+					[y] = z;
+				}
+				new Foo()
+			`,
+		},
+		entryPaths: []string{"/entry.ts"},
+		options: config.Options{
+			Mode:                    config.ModePassThrough,
+			AbsOutputFile:           "/out.js",
+			UseDefineForClassFields: config.True,
+		},
+	})
+}
+
+func TestTSComputedClassFieldUseDefineTrueLower(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.ts": `
+				class Foo {
+					[q];
+					[r] = s;
+					@dec
+					[x];
+					@dec
+					[y] = z;
+				}
+				new Foo()
+			`,
+		},
+		entryPaths: []string{"/entry.ts"},
+		options: config.Options{
+			Mode:                    config.ModePassThrough,
+			AbsOutputFile:           "/out.js",
+			UseDefineForClassFields: config.True,
+			UnsupportedJSFeatures:   compat.ClassField,
+		},
+	})
+}

internal/bundler/snapshots/snapshots_default.txt

@@ -2564,6 +2564,64 @@ switch (bar) {
     let a;
 }
 
+================================================================================
+TestTSComputedClassFieldUseDefineFalse
+---------- /out.js ----------
+var _a, _b, _c;
+class Foo {
+  constructor() {
+    this[_a] = s;
+    this[_c] = z;
+  }
+}
+q, _a = r, _b = x, _c = y;
+__decorateClass([
+  dec
+], Foo.prototype, _b, 2);
+__decorateClass([
+  dec
+], Foo.prototype, _c, 2);
+new Foo();
+
+================================================================================
+TestTSComputedClassFieldUseDefineTrue
+---------- /out.js ----------
+var _a, _b;
+class Foo {
+  [q];
+  [r] = s;
+  [_a = x];
+  [_b = y] = z;
+}
+__decorateClass([
+  dec
+], Foo.prototype, _a, 2);
+__decorateClass([
+  dec
+], Foo.prototype, _b, 2);
+new Foo();
+
+================================================================================
+TestTSComputedClassFieldUseDefineTrueLower
+---------- /out.js ----------
+var _a, _b, _c, _d;
+class Foo {
+  constructor() {
+    __publicField(this, _a);
+    __publicField(this, _b, s);
+    __publicField(this, _c);
+    __publicField(this, _d, z);
+  }
+}
+_a = q, _b = r, _c = x, _d = y;
+__decorateClass([
+  dec
+], Foo.prototype, _c, 2);
+__decorateClass([
+  dec
+], Foo.prototype, _d, 2);
+new Foo();
+
 ================================================================================
 TestThisInsideFunction
 ---------- /out.js ----------

internal/js_parser/js_parser_lower.go

@@ -2070,7 +2070,7 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, shadowRef js_ast
 		// Make sure the order of computed property keys doesn't change. These
 		// expressions have side effects and must be evaluated in order.
 		keyExprNoSideEffects := prop.Key
-		if prop.IsComputed && (p.options.ts.Parse || len(prop.TSDecorators) > 0 ||
+		if prop.IsComputed && (len(prop.TSDecorators) > 0 ||
 			mustLowerField || computedPropertyCache.Data != nil) {
 			needsKey := true
 			if len(prop.TSDecorators) == 0 && (prop.IsMethod || shouldOmitFieldInitializer) {
@@ -2092,7 +2092,7 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, shadowRef js_ast
 			// If this is a computed method, the property value will be used
 			// immediately. In this case we inline all computed properties so far to
 			// make sure all computed properties before this one are evaluated first.
-			if prop.IsMethod {
+			if !mustLowerField {
 				prop.Key = computedPropertyCache
 				computedPropertyCache = js_ast.Expr{}
 			}