fix #2158: private+super+static+bundle edge case

Author: evanw

CHANGELOG.md

@@ -2,6 +2,65 @@
 
 ## Unreleased
 
+* Fix a regression regarding `super` ([#2158](https://github.com/evanw/esbuild/issues/2158))
+
+    This fixes a regression from the previous release regarding classes with a super class, a private member, and a static field in the scenario where the static field needs to be lowered but where private members are supported by the configured target environment. In this scenario, esbuild could incorrectly inject the instance field initializers that use `this` into the constructor before the call to `super()`, which is invalid. This problem has now been fixed (notice that `this` is now used after `super()` instead of before):
+
+    ```js
+    // Original code
+    class Foo extends Object {
+      static FOO;
+      constructor() {
+        super();
+      }
+      #foo;
+    }
+
+    // Old output (with --bundle)
+    var _foo;
+    var Foo = class extends Object {
+      constructor() {
+        __privateAdd(this, _foo, void 0);
+        super();
+      }
+    };
+    _foo = new WeakMap();
+    __publicField(Foo, "FOO");
+
+    // New output (with --bundle)
+    var _foo;
+    var Foo = class extends Object {
+      constructor() {
+        super();
+        __privateAdd(this, _foo, void 0);
+      }
+    };
+    _foo = new WeakMap();
+    __publicField(Foo, "FOO");
+    ```
+
+    During parsing, esbuild scans the class and makes certain decisions about the class such as whether to lower all static fields, whether to lower each private member, or whether calls to `super()` need to be tracked and adjusted. Previously esbuild made two passes through the class members to compute this information. However, with the new `super()` call lowering logic added in the previous release, we now need three passes to capture the whole dependency chain for this case: 1) lowering static fields requires 2) lowering private members which requires 3) adjusting `super()` calls.
+
+    The reason lowering static fields requires lowering private members is because lowering static fields moves their initializers outside of the class body, where they can't access private members anymore. Consider this code:
+
+    ```js
+    class Foo {
+      get #foo() {}
+      static bar = new Foo().#foo
+    }
+    ```
+
+    We can't just lower static fields without also lowering private members, since that causes a syntax error:
+
+    ```js
+    class Foo {
+      get #foo() {}
+    }
+    Foo.bar = new Foo().#foo;
+    ```
+
+    And the reason lowering private members requires adjusting `super()` calls is because the injected private member initializers use `this`, which is only accessible after `super()` calls in the constructor.
+
 * Add Linux ARM64 support for Deno ([#2156](https://github.com/evanw/esbuild/issues/2156))
 
     This release adds Linux ARM64 support to esbuild's [Deno](https://deno.land/) API implementation, which allows esbuild to be used with Deno on a Raspberry Pi.

internal/bundler/bundler_lower_test.go

@@ -1532,6 +1532,28 @@ func TestLowerPrivateSuperES2021(t *testing.T) {
 	})
 }
 
+// https://github.com/evanw/esbuild/issues/2158
+func TestLowerPrivateSuperStaticBundleIssue2158(t *testing.T) {
+	lower_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				export class Foo extends Object {
+					static FOO;
+					constructor() {
+						super();
+					}
+					#foo;
+				}
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}
+
 func TestLowerClassField2020NoBundle(t *testing.T) {
 	lower_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/bundler/snapshots/snapshots_lower.txt

@@ -1633,6 +1633,23 @@ export {
   foo8_default as foo8
 };
 
+================================================================================
+TestLowerPrivateSuperStaticBundleIssue2158
+---------- /out.js ----------
+// entry.js
+var _foo;
+var Foo = class extends Object {
+  constructor() {
+    super();
+    __privateAdd(this, _foo, void 0);
+  }
+};
+_foo = new WeakMap();
+__publicField(Foo, "FOO");
+export {
+  Foo
+};
+
 ================================================================================
 TestLowerStaticAsyncArrowSuperES2016
 ---------- /out.js ----------

internal/js_parser/js_parser.go

@@ -10158,6 +10158,7 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul
 	// field initializers outside of the class body and "this" will no longer
 	// reference the same thing.
 	classLoweringInfo := p.computeClassLoweringInfo(class)
+	recomputeClassLoweringInfo := false
 
 	// Sometimes we need to lower private members even though they are supported.
 	// This flags them for lowering so that we lower references to them as we
@@ -10186,6 +10187,7 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul
 			// The private getter must be lowered too.
 			if private, ok := prop.Key.Data.(*js_ast.EPrivateIdentifier); ok {
 				p.symbols[private.Ref.InnerIndex].Flags |= js_ast.PrivateSymbolMustBeLowered
+				recomputeClassLoweringInfo = true
 			}
 		}
 	}
@@ -10197,11 +10199,18 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul
 			if private, ok := prop.Key.Data.(*js_ast.EPrivateIdentifier); ok {
 				if symbol := &p.symbols[private.Ref.InnerIndex]; p.classPrivateBrandChecksToLower[symbol.OriginalName] {
 					symbol.Flags |= js_ast.PrivateSymbolMustBeLowered
+					recomputeClassLoweringInfo = true
 				}
 			}
 		}
 	}
 
+	// If we changed private symbol lowering decisions, then recompute class
+	// lowering info because that may have changed other decisions too
+	if recomputeClassLoweringInfo {
+		classLoweringInfo = p.computeClassLoweringInfo(class)
+	}
+
 	p.pushScopeForVisitPass(js_ast.ScopeClassName, nameScopeLoc)
 	oldEnclosingClassKeyword := p.enclosingClassKeyword
 	p.enclosingClassKeyword = class.ClassKeyword

scripts/end-to-end-tests.js

@@ -4923,6 +4923,26 @@
           }
         `,
       }, { async: true }),
+      test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
+        // Check edge case in https://github.com/evanw/esbuild/issues/2158
+        'in.js': `
+        class Foo {
+          constructor(x) {
+            this.base = x
+          }
+        }
+        class Bar extends Foo {
+          static FOO = 1
+          constructor() {
+            super(2)
+            this.derived = this.#foo + Bar.FOO
+          }
+          #foo = 3
+        }
+        let bar = new Bar
+        if (bar.base !== 2 || bar.derived !== 4) throw 'fail'
+        `,
+      }),
       test(['in.js', '--outfile=node.js', '--keep-names', '--bundle'].concat(flags), {
         // Check default export name preservation with lowered "super" inside lowered "async"
         'in.js': `