fix #3768: bundled decorators in derived classes

evanw · May 15, 2024 · 66b7c6d · 66b7c6d
1 parent 6e6f15f
commit 66b7c6d
Show file tree

Hide file tree

Showing 5 changed files with 248 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,19 @@
 
 ## Unreleased
 
+* Fix bundled decorators in derived classes ([#3768](https://github.com/evanw/esbuild/issues/3768))
+
+    In certain cases, bundling code that uses decorators in a derived class with a class body that references its own class name could previously generate code that crashes at run-time due to an incorrect variable name. This problem has been fixed. Here is an example of code that was compiled incorrectly before this fix:
+
+    ```js
+    class Foo extends Object {
+      @(x => x) foo() {
+        return Foo
+      }
+    }
+    console.log(new Foo().foo())
+    ```
+
 * Fix `tsconfig.json` files inside symlinked directories ([#3767](https://github.com/evanw/esbuild/issues/3767))
 
     This release fixes an issue with a scenario involving a `tsconfig.json` file that `extends` another file from within a symlinked directory that uses the `paths` feature. In that case, the implicit `baseURL` value should be based on the real path (i.e. after expanding all symbolic links) instead of the original path. This was already done for other files that esbuild resolves but was not yet done for `tsconfig.json` because it's special-cased (the regular path resolver can't be used because the information inside `tsconfig.json` is involved in path resolution). Note that this fix no longer applies if the `--preserve-symlinks` setting is enabled.

diff --git a/internal/bundler_tests/bundler_lower_test.go b/internal/bundler_tests/bundler_lower_test.go
@@ -3162,3 +3162,31 @@ func TestLowerAsyncGeneratorNoAwait(t *testing.T) {
 		},
 	})
 }
+
+func TestJavaScriptDecoratorsBundleIssue3768(t *testing.T) {
+	lower_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/base-instance-method.js":   `class Foo { @dec foo() { return Foo } }`,
+			"/base-instance-field.js":    `class Foo { @dec foo = Foo }`,
+			"/base-instance-accessor.js": `class Foo { @dec accessor foo = Foo }`,
+
+			"/base-static-method.js":   `class Foo { @dec static foo() { return Foo } }`,
+			"/base-static-field.js":    `class Foo { @dec static foo = Foo }`,
+			"/base-static-accessor.js": `class Foo { @dec static accessor foo = Foo }`,
+
+			"/derived-instance-method.js":   `class Foo extends Bar { @dec foo() { return Foo } }`,
+			"/derived-instance-field.js":    `class Foo extends Bar { @dec foo = Foo }`,
+			"/derived-instance-accessor.js": `class Foo extends Bar { @dec accessor foo = Foo }`,
+
+			"/derived-static-method.js":   `class Foo extends Bar { @dec static foo() { return Foo } }`,
+			"/derived-static-field.js":    `class Foo extends Bar { @dec static foo = Foo }`,
+			"/derived-static-accessor.js": `class Foo extends Bar { @dec static accessor foo = Foo }`,
+		},
+		entryPaths: []string{"/*"},
+		options: config.Options{
+			Mode:                  config.ModeBundle,
+			AbsOutputDir:          "/out",
+			UnsupportedJSFeatures: compat.Decorators,
+		},
+	})
+}
diff --git a/internal/bundler_tests/snapshots/snapshots_lower.txt b/internal/bundler_tests/snapshots/snapshots_lower.txt
@@ -619,6 +619,165 @@ class StaticPrivate {
   }
 }
 
+================================================================================
+TestJavaScriptDecoratorsBundleIssue3768
+---------- /out/base-instance-accessor.js ----------
+// base-instance-accessor.js
+var _foo_dec, _init, _foo;
+_init = [, , ,];
+_foo_dec = [dec];
+var _Foo = class _Foo {
+  constructor() {
+    __privateAdd(this, _foo, __runInitializers(_init, 6, this, _Foo)), __runInitializers(_init, 9, this);
+  }
+};
+_foo = new WeakMap();
+__decorateElement(_init, 4, "foo", _foo_dec, _Foo, _foo);
+var Foo = _Foo;
+
+---------- /out/base-instance-field.js ----------
+// base-instance-field.js
+var _foo_dec, _init;
+_init = [, , ,];
+_foo_dec = [dec];
+var _Foo = class _Foo {
+  constructor() {
+    __publicField(this, "foo", __runInitializers(_init, 6, this, _Foo)), __runInitializers(_init, 9, this);
+  }
+};
+__decorateElement(_init, 5, "foo", _foo_dec, _Foo);
+var Foo = _Foo;
+
+---------- /out/base-instance-method.js ----------
+// base-instance-method.js
+var _foo_dec, _init;
+_init = [, , ,];
+_foo_dec = [dec];
+var _Foo = class _Foo {
+  constructor() {
+    __runInitializers(_init, 5, this);
+  }
+  foo() {
+    return _Foo;
+  }
+};
+__decorateElement(_init, 1, "foo", _foo_dec, _Foo);
+var Foo = _Foo;
+
+---------- /out/base-static-accessor.js ----------
+// base-static-accessor.js
+var _foo_dec, _init, _foo;
+_init = [, , ,];
+_foo_dec = [dec];
+var _Foo = class _Foo {
+};
+_foo = new WeakMap();
+__decorateElement(_init, 12, "foo", _foo_dec, _Foo, _foo);
+__privateAdd(_Foo, _foo, __runInitializers(_init, 6, _Foo, _Foo)), __runInitializers(_init, 9, _Foo);
+var Foo = _Foo;
+
+---------- /out/base-static-field.js ----------
+// base-static-field.js
+var _foo_dec, _init;
+_init = [, , ,];
+_foo_dec = [dec];
+var _Foo = class _Foo {
+};
+__decorateElement(_init, 13, "foo", _foo_dec, _Foo);
+__publicField(_Foo, "foo", __runInitializers(_init, 6, _Foo, _Foo)), __runInitializers(_init, 9, _Foo);
+var Foo = _Foo;
+
+---------- /out/base-static-method.js ----------
+// base-static-method.js
+var _foo_dec, _init;
+_init = [, , ,];
+_foo_dec = [dec];
+var _Foo = class _Foo {
+  static foo() {
+    return _Foo;
+  }
+};
+__decorateElement(_init, 9, "foo", _foo_dec, _Foo);
+__runInitializers(_init, 3, _Foo);
+var Foo = _Foo;
+
+---------- /out/derived-instance-accessor.js ----------
+// derived-instance-accessor.js
+var _foo_dec, _a, _init, _foo;
+_init = [, , ,];
+var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
+  constructor() {
+    super(...arguments);
+    __privateAdd(this, _foo, __runInitializers(_init, 6, this, _Foo)), __runInitializers(_init, 9, this);
+  }
+};
+_foo = new WeakMap();
+__decorateElement(_init, 4, "foo", _foo_dec, _Foo, _foo);
+var Foo = _Foo;
+
+---------- /out/derived-instance-field.js ----------
+// derived-instance-field.js
+var _foo_dec, _a, _init;
+_init = [, , ,];
+var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
+  constructor() {
+    super(...arguments);
+    __publicField(this, "foo", __runInitializers(_init, 6, this, _Foo)), __runInitializers(_init, 9, this);
+  }
+};
+__decorateElement(_init, 5, "foo", _foo_dec, _Foo);
+var Foo = _Foo;
+
+---------- /out/derived-instance-method.js ----------
+// derived-instance-method.js
+var _foo_dec, _a, _init;
+_init = [, , ,];
+var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
+  constructor() {
+    super(...arguments);
+    __runInitializers(_init, 5, this);
+  }
+  foo() {
+    return _Foo;
+  }
+};
+__decorateElement(_init, 1, "foo", _foo_dec, _Foo);
+var Foo = _Foo;
+
+---------- /out/derived-static-accessor.js ----------
+// derived-static-accessor.js
+var _foo_dec, _a, _init, _foo;
+_init = [, , ,];
+var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
+};
+_foo = new WeakMap();
+__decorateElement(_init, 12, "foo", _foo_dec, _Foo, _foo);
+__privateAdd(_Foo, _foo, __runInitializers(_init, 6, _Foo, _Foo)), __runInitializers(_init, 9, _Foo);
+var Foo = _Foo;
+
+---------- /out/derived-static-field.js ----------
+// derived-static-field.js
+var _foo_dec, _a, _init;
+_init = [, , ,];
+var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
+};
+__decorateElement(_init, 13, "foo", _foo_dec, _Foo);
+__publicField(_Foo, "foo", __runInitializers(_init, 6, _Foo, _Foo)), __runInitializers(_init, 9, _Foo);
+var Foo = _Foo;
+
+---------- /out/derived-static-method.js ----------
+// derived-static-method.js
+var _foo_dec, _a, _init;
+_init = [, , ,];
+var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
+  static foo() {
+    return _Foo;
+  }
+};
+__decorateElement(_init, 9, "foo", _foo_dec, _Foo);
+__runInitializers(_init, 3, _Foo);
+var Foo = _Foo;
+
 ================================================================================
 TestJavaScriptDecoratorsESNext
 ---------- /out.js ----------

diff --git a/internal/js_parser/js_parser_lower_class.go b/internal/js_parser/js_parser_lower_class.go
@@ -2110,9 +2110,14 @@ func (ctx *lowerClassContext) finishAndGenerateCode(p *parser, result visitClass
 			len(ctx.privateMembers) > 0 ||
 			len(ctx.staticPrivateMethods) > 0 ||
 			len(ctx.staticMembers) > 0 ||
+
+			// TypeScript experimental decorators
 			len(ctx.instanceExperimentalDecorators) > 0 ||
 			len(ctx.staticExperimentalDecorators) > 0 ||
-			len(classExperimentalDecorators) > 0)
+			len(classExperimentalDecorators) > 0 ||
+
+			// JavaScript decorators
+			ctx.decoratorContextRef != ast.InvalidRef)
 
 	// If we need to represent the class as an expression (even if it's a
 	// statement), then generate another symbol to use as the class name

diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js
@@ -6087,6 +6087,48 @@ for (let flags of [['--target=es2022'], ['--target=es6'], ['--bundle', '--target
       }`,
     }),
   )
+
+  // https://github.com/evanw/esbuild/issues/3768
+  tests.push(
+    test(['in.ts', '--outfile=node.js'].concat(flags), {
+      'in.ts': `
+        const bar = x => x
+        class Foo {
+          @bar baz() { return Foo }
+        }
+        if (new Foo().baz() !== Foo) throw 'fail'
+      `,
+    }),
+    test(['in.ts', '--outfile=node.js'].concat(flags), {
+      'in.ts': `
+        class Foo {}
+        const bar = x => x
+        class Baz extends Foo {
+          @bar baz() { return Baz }
+        }
+        if (new Baz().baz() !== Baz) throw 'fail'
+      `,
+    }),
+    test(['in.ts', '--outfile=node.js'].concat(flags), {
+      'in.ts': `
+        const bar = () => x => x
+        class Foo {
+          @bar baz = Foo
+        }
+        if (new Foo().baz !== Foo) throw 'fail'
+      `,
+    }),
+    test(['in.ts', '--outfile=node.js'].concat(flags), {
+      'in.ts': `
+        class Foo {}
+        const bar = () => x => x
+        class Baz extends Foo {
+          @bar baz = Baz
+        }
+        if (new Baz().baz !== Baz) throw 'fail'
+      `,
+    }),
+  )
 }
 
 // Async lowering tests