fix #2389: use a static block for --keep-names

CHANGELOG.md

@@ -31,6 +31,41 @@
     __publicField(Foo, "y", console.log(3));
     ```
 
+* Use static blocks to implement `--keep-names` on classes ([#2389](https://github.com/evanw/esbuild/issues/2389))
+
+    This change fixes a bug where the `name` property could previously be incorrect within a class static context when using `--keep-names`. The problem was that the `name` property was being initialized after static blocks were run instead of before. This has been fixed by moving the `name` property initializer into a static block at the top of the class body:
+
+    ```js
+    // Original code
+    if (typeof Foo === 'undefined') {
+      let Foo = class {
+        static test = this.name
+      }
+      console.log(Foo.test)
+    }
+
+    // Old output (with --keep-names)
+    if (typeof Foo === "undefined") {
+      let Foo2 = /* @__PURE__ */ __name(class {
+        static test = this.name;
+      }, "Foo");
+      console.log(Foo2.test);
+    }
+
+    // New output (with --keep-names)
+    if (typeof Foo === "undefined") {
+      let Foo2 = class {
+        static {
+          __name(this, "Foo");
+        }
+        static test = this.name;
+      };
+      console.log(Foo2.test);
+    }
+    ```
+
+    This change was somewhat involved, especially regarding what esbuild considers to be side-effect free. Some unused classes that weren't removed by tree shaking in previous versions of esbuild may now be tree-shaken. One example is classes with static private fields that are transformed by esbuild into code that doesn't use JavaScript's private field syntax. Previously esbuild's tree shaking analysis ran on the class after syntax lowering, but with this release it will run on the class before syntax lowering, meaning it should no longer be confused by class mutations resulting from automatically-generated syntax lowering code.
+
 ## 0.18.1
 
 * Fill in `null` entries in input source maps ([#3144](https://github.com/evanw/esbuild/issues/3144))

internal/bundler_tests/bundler_lower_test.go

@@ -384,10 +384,10 @@ func TestTSLowerPrivateFieldAndMethodAvoidNameCollision2015(t *testing.T) {
 	lower_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.ts": `
-				class WeakMap {
+				export class WeakMap {
 					#x
 				}
-				class WeakSet {
+				export class WeakSet {
 					#y() {}
 				}
 			`,
@@ -405,7 +405,7 @@ func TestLowerPrivateGetterSetter2015(t *testing.T) {
 	lower_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
-				class Foo {
+				export class Foo {
 					get #foo() { return this.foo }
 					set #bar(val) { this.bar = val }
 					get #prop() { return this.prop }
@@ -456,7 +456,7 @@ func TestLowerPrivateGetterSetter2019(t *testing.T) {
 	lower_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
-				class Foo {
+				export class Foo {
 					get #foo() { return this.foo }
 					set #bar(val) { this.bar = val }
 					get #prop() { return this.prop }
@@ -507,7 +507,7 @@ func TestLowerPrivateGetterSetter2020(t *testing.T) {
 	lower_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
-				class Foo {
+				export class Foo {
 					get #foo() { return this.foo }
 					set #bar(val) { this.bar = val }
 					get #prop() { return this.prop }
@@ -608,7 +608,7 @@ func TestLowerPrivateMethod2019(t *testing.T) {
 	lower_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
-				class Foo {
+				export class Foo {
 					#field
 					#method() {}
 					baseline() {
@@ -650,7 +650,7 @@ func TestLowerPrivateMethod2020(t *testing.T) {
 	lower_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
-				class Foo {
+				export class Foo {
 					#field
 					#method() {}
 					baseline() {
@@ -757,7 +757,7 @@ func TestLowerPrivateMethodWithModifiers2020(t *testing.T) {
 	lower_suite.expectBundled(t, bundled{
 		files: map[string]string{
 			"/entry.js": `
-				class Foo {
+				export class Foo {
 					*#g() {}
 					async #a() {}
 					async *#ag() {}

internal/bundler_tests/bundler_ts_test.go

@@ -1057,7 +1057,7 @@ func TestTSExperimentalDecoratorsKeepNames(t *testing.T) {
 		files: map[string]string{
 			"/entry.ts": `
 				@decoratorMustComeAfterName
-				class Foo {}
+				export class Foo {}
 			`,
 			"/tsconfig.json": `{
 				"compilerOptions": {

internal/bundler_tests/snapshots/snapshots_default.txt

@@ -2408,9 +2408,11 @@ console.log([
 TestKeepNamesClassStaticName
 ---------- /out.js ----------
 class A {
+  static {
+    __name(this, "A");
+  }
   static foo;
 }
-__name(A, "A");
 class B {
   static name;
 }
@@ -2429,9 +2431,12 @@ class E {
 class F {
   static ["name"] = 0;
 }
-let a = /* @__PURE__ */ __name(class a3 {
+let a = class a3 {
+  static {
+    __name(this, "a");
+  }
   static foo;
-}, "a");
+};
 let b = class b3 {
   static name;
 };
@@ -2450,9 +2455,12 @@ let e = class e3 {
 let f = class f3 {
   static ["name"] = 0;
 };
-let a2 = /* @__PURE__ */ __name(class {
+let a2 = class {
+  static {
+    __name(this, "a2");
+  }
   static foo;
-}, "a2");
+};
 let b2 = class {
   static name;
 };
@@ -2476,19 +2484,22 @@ let f2 = class {
 TestKeepNamesTreeShaking
 ---------- /out.js ----------
 // entry.js
+function fnStmtRemove() {
+}
+__name(fnStmtRemove, "fnStmtRemove");
 function fnStmtKeep() {
 }
 __name(fnStmtKeep, "fnStmtKeep");
 x = fnStmtKeep;
 var fnExprKeep = /* @__PURE__ */ __name(function() {
 }, "keep");
 x = fnExprKeep;
-var clsStmtKeep = class {
-};
-__name(clsStmtKeep, "clsStmtKeep");
+var _clsStmtKeep = class {
+}, clsStmtKeep = _clsStmtKeep;
+__name(_clsStmtKeep, "clsStmtKeep");
 new clsStmtKeep();
-var clsExprKeep = /* @__PURE__ */ __name(class {
-}, "keep");
+var _a, clsExprKeep = (_a = class {
+}, __name(_a, "keep"), _a);
 new clsExprKeep();
 
 ================================================================================
@@ -2947,9 +2958,7 @@ var { "_doNotMangleThis": x } = y;
 ================================================================================
 TestMangleNoQuotedPropsMinifySyntax
 ---------- /out/entry.js ----------
-x._doNotMangleThis, x?._doNotMangleThis, x[y ? "_doNotMangleThis" : z], x?.[y ? "_doNotMangleThis" : z], x[y ? z : "_doNotMangleThis"], x?.[y ? z : "_doNotMangleThis"], class {
-  _doNotMangleThis = x;
-};
+x._doNotMangleThis, x?._doNotMangleThis, x[y ? "_doNotMangleThis" : z], x?.[y ? "_doNotMangleThis" : z], x[y ? z : "_doNotMangleThis"], x?.[y ? z : "_doNotMangleThis"];
 var { _doNotMangleThis: x } = y;
 "_doNotMangleThis" in x, (y ? "_doNotMangleThis" : z) in x, (y ? z : "_doNotMangleThis") in x;
 
@@ -3415,10 +3424,6 @@ foo("_keepThisProperty") in x;
 
 ---------- /out/mangle.js ----------
 x.a, x?.a, x[y ? "a" : z], x?.[y ? "a" : z], x[y ? z : "a"], x?.[y ? z : "a"], x[y, "a"], x?.[y, "a"], (y, "a") + "", class {
-  a = x;
-}, class {
-  a = x;
-}, class {
   [(y, "a")] = x;
 };
 var { a: x } = y, { ["a"]: x } = y, { [(z, "a")]: x } = y;

internal/bundler_tests/snapshots/snapshots_lower.txt

@@ -1326,6 +1326,9 @@ prop_get = function() {
 prop_set = function(val) {
   this.prop = val;
 };
+export {
+  Foo
+};
 
 ================================================================================
 TestLowerPrivateGetterSetter2019
@@ -1385,6 +1388,9 @@ prop_get = function() {
 prop_set = function(val) {
   this.prop = val;
 };
+export {
+  Foo
+};
 
 ================================================================================
 TestLowerPrivateGetterSetter2020
@@ -1444,6 +1450,9 @@ prop_get = function() {
 prop_set = function(val) {
   this.prop = val;
 };
+export {
+  Foo
+};
 
 ================================================================================
 TestLowerPrivateGetterSetterNext
@@ -1538,6 +1547,9 @@ _field = new WeakMap();
 _method = new WeakSet();
 method_fn = function() {
 };
+export {
+  Foo
+};
 
 ================================================================================
 TestLowerPrivateMethod2020
@@ -1579,6 +1591,9 @@ _field = new WeakMap();
 _method = new WeakSet();
 method_fn = function() {
 };
+export {
+  Foo
+};
 
 ================================================================================
 TestLowerPrivateMethodNext
@@ -1649,6 +1664,9 @@ sag_fn = async function* () {
 __privateAdd(Foo, _sg);
 __privateAdd(Foo, _sa);
 __privateAdd(Foo, _sag);
+export {
+  Foo
+};
 
 ================================================================================
 TestLowerPrivateSuperES2021
@@ -2596,6 +2614,10 @@ var WeakSet2 = class {
 _y = new WeakSet();
 y_fn = function() {
 };
+export {
+  WeakMap2 as WeakMap,
+  WeakSet2 as WeakSet
+};
 
 ================================================================================
 TestTSLowerPrivateFieldOptionalChain2015NoBundle

internal/bundler_tests/snapshots/snapshots_ts.txt

@@ -892,6 +892,9 @@ __name(Foo, "Foo");
 Foo = __decorateClass([
   decoratorMustComeAfterName
 ], Foo);
+export {
+  Foo
+};
 
 ================================================================================
 TestTSExportDefaultTypeIssue316

internal/js_ast/js_ast.go

@@ -357,6 +357,28 @@ type Class struct {
 	ClassKeyword  logger.Range
 	BodyLoc       logger.Loc
 	CloseBraceLoc logger.Loc
+
+	// If true, property field initializers cannot be assumed to have no side
+	// effects. For example:
+	//
+	//   class Foo {
+	//     static set foo(x) { importantSideEffect(x) }
+	//   }
+	//   class Bar extends Foo {
+	//     foo = 1
+	//   }
+	//
+	// This happens in TypeScript when "useDefineForClassFields" is disabled
+	// because TypeScript (and esbuild) transforms the above class into this:
+	//
+	//   class Foo {
+	//     static set foo(x) { importantSideEffect(x); }
+	//   }
+	//   class Bar extends Foo {
+	//   }
+	//   Bar.foo = 1;
+	//
+	UseDefineForClassFields bool
 }
 
 type ArrayBinding struct {
@@ -436,6 +458,7 @@ func (*EString) isExpr()               {}
 func (*ETemplate) isExpr()             {}
 func (*ERegExp) isExpr()               {}
 func (*EInlinedEnum) isExpr()          {}
+func (*EAnnotation) isExpr()           {}
 func (*EAwait) isExpr()                {}
 func (*EYield) isExpr()                {}
 func (*EIf) isExpr()                   {}
@@ -545,7 +568,6 @@ const (
 	NormalCall CallKind = iota
 	DirectEval
 	TargetWasOriginallyPropertyAccess
-	InternalPublicFieldCall
 )
 
 type OptionalChain uint8
@@ -791,6 +813,24 @@ type EInlinedEnum struct {
 	Comment string
 }
 
+type AnnotationFlags uint8
+
+const (
+	// This is sort of like an IIFE with a "/* @__PURE__ */" comment except it's an
+	// inline annotation on an expression itself without the nested scope. Sometimes
+	// we can't easily introduce a new scope (e.g. if the expression uses "await").
+	CanBeRemovedIfUnusedFlag AnnotationFlags = 1 << iota
+)
+
+func (flags AnnotationFlags) Has(flag AnnotationFlags) bool {
+	return (flags & flag) != 0
+}
+
+type EAnnotation struct {
+	Value Expr
+	Flags AnnotationFlags
+}
+
 type EAwait struct {
 	Value Expr
 }
@@ -938,10 +978,13 @@ type SLazyExport struct {
 type SExpr struct {
 	Value Expr
 
-	// This is set to true for automatically-generated expressions that should
-	// not affect tree shaking. For example, calling a function from the runtime
-	// that doesn't have externally-visible side effects.
-	DoesNotAffectTreeShaking bool
+	// This is set to true for automatically-generated expressions that are part
+	// of class syntax lowering. A single class declaration may end up with many
+	// generated expressions after it (e.g. class field initializations, a call
+	// to keep the original value of the "name" property). When this happens we
+	// can't tell that the class is side-effect free anymore because all of these
+	// methods mutate the class. We use this annotation for that instead.
+	IsFromClassThatCanBeRemovedIfUnused bool
 }
 
 type EnumValue struct {