fix #2147: ts decorators use scope enclosing class

CHANGELOG.md

@@ -2,53 +2,88 @@
 
 ## Unreleased
 
-* Change the context of TypeScript parameter decorators
+* Change the context of TypeScript parameter decorators ([#2147](https://github.com/evanw/esbuild/issues/2147))
 
     While TypeScript parameter decorators are expressions, they are not evaluated where they exist in the code. They are moved to after the class declaration and evaluated there instead. Specifically this TypeScript code:
 
     ```ts
-    class Foo {
-      foo(@bar() baz) {}
+    class Class {
+      method(@decorator() arg) {}
     }
     ```
 
     becomes this JavaScript code:
 
     ```js
-    class Foo {
-      foo(baz) {}
+    class Class {
+      method(arg) {}
     }
     __decorate([
-      __param(0, bar())
-    ], Foo.prototype, "foo", null);
+      __param(0, decorator())
+    ], Class.prototype, "method", null);
     ```
 
-    One consequence of this is that whether `await` is allowed or not depends on whether the class declaration itself is inside an `async` function or not. The TypeScript compiler allows code that does this:
+    This has several consequences:
 
-    ```ts
-    async function fn(foo) {
-      class Foo {
-        foo(@bar(await foo) baz) {}
-      }
-      return Foo
-    }
-    ```
+    * Whether `await` is allowed inside a decorator expression or not depends on whether the class declaration itself is in an `async` context or not. With this release, you can now use `await` inside a decorator expression when the class declaration is either inside an `async` function or is at the top-level of an ES module and top-level await is supported. Note that the TypeScript compiler currently has a bug regarding this edge case: https://github.com/microsoft/TypeScript/issues/48509.
+
+        ```ts
+        // Using "await" inside a decorator expression is now allowed
+        async function fn(foo: Promise<any>) {
+          class Class {
+            method(@decorator(await foo) arg) {}
+          }
+          return Class
+        }
+        ```
 
-    because that becomes the following valid JavaScript:
+        Also while TypeScript currently allows `await` to be used like this in `async` functions, it doesn't currently allow `yield` to be used like this in generator functions. It's not yet clear whether this behavior with `yield` is a bug or by design, so I haven't made any changes to esbuild's handling of `yield` inside decorator expressions in this release.
 
-    ```js
-    async function fn(foo) {
-      class Foo {
-        foo(baz) {}
-      }
-      __decorate([
-        __param(0, bar(await foo))
-      ], Foo.prototype, "foo", null);
-      return Foo;
-    }
-    ```
+    * Since the scope of a decorator expression is the scope enclosing the class declaration, they cannot access private identifiers. Previously this was incorrectly allowed but with this release, esbuild no longer allows this. Note that the TypeScript compiler currently has a bug regarding this edge case: https://github.com/microsoft/TypeScript/issues/48515.
 
-    Previously using `await` like this wasn't allowed. With this release, esbuild now handles `await` correctly in TypeScript parameter decorators. Note that the TypeScript compiler currently has some bugs regarding this behavior: https://github.com/microsoft/TypeScript/issues/48509. Also while TypeScript currently allows `await` to be used like this in `async` functions, it doesn't currently allow `yield` to be used like this in generator functions. It's not yet clear whether this behavior with `yield` is a bug or by design, so I haven't made any changes to esbuild's handling of `yield` inside decorator expressions.
+        ```ts
+        // Using private names inside a decorator expression is no longer allowed
+        class Class {
+          static #priv = 123
+          method(@decorator(Class.#priv) arg) {}
+        }
+        ```
+
+    * Since the scope of a decorator expression is the scope enclosing the class declaration, identifiers inside parameter decorator expressions should never be resolved to a parameter of the enclosing method. Previously this could happen, which was a bug with esbuild. This bug no longer happens in this release.
+
+        ```ts
+        // Name collisions now resolve to the outer name instead of the inner name
+        let arg = 1
+        class Class {
+          method(@decorator(arg) arg = 2) {}
+        }
+        ```
+
+        Specifically previous versions of esbuild generated the following incorrect JavaScript (notice the use of `arg2`):
+
+        ```js
+        let arg = 1;
+        class Class {
+          method(arg2 = 2) {
+          }
+        }
+        __decorateClass([
+          __decorateParam(0, decorator(arg2))
+        ], Class.prototype, "method", 1);
+        ```
+
+        This release now generates the following correct JavaScript (notice the use of `arg`):
+
+        ```js
+        let arg = 1;
+        class Class {
+          method(arg2 = 2) {
+          }
+        }
+        __decorateClass([
+          __decorateParam(0, decorator(arg))
+        ], Class.prototype, "method", 1);
+        ```
 
 ## 0.14.29
 

internal/bundler/bundler_ts_test.go

@@ -864,6 +864,43 @@ func TestTypeScriptDecoratorsKeepNames(t *testing.T) {
 	})
 }
 
+// See: https://github.com/evanw/esbuild/issues/2147
+func TestTypeScriptDecoratorScopeIssue2147(t *testing.T) {
+	ts_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.ts": `
+				let foo = 1
+				class Foo {
+					method1(@dec(foo) foo = 2) {}
+					method2(@dec(() => foo) foo = 3) {}
+				}
+
+				class Bar {
+					static x = class {
+						static y = () => {
+							let bar = 1
+							@dec(bar)
+							@dec(() => bar)
+							class Baz {
+								@dec(bar) method1() {}
+								@dec(() => bar) method2() {}
+								method3(@dec(() => bar) bar) {}
+								method4(@dec(() => bar) bar) {}
+							}
+							return Baz
+						}
+					}
+				}
+			`,
+		},
+		entryPaths: []string{"/entry.ts"},
+		options: config.Options{
+			Mode:          config.ModePassThrough,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}
+
 func TestTSExportDefaultTypeIssue316(t *testing.T) {
 	ts_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/bundler/snapshots/snapshots_ts.txt

@@ -832,6 +832,57 @@ export var x;
   x2.z = x2.y;
 })(x || (x = {}));
 
+================================================================================
+TestTypeScriptDecoratorScopeIssue2147
+---------- /out.js ----------
+var _a;
+let foo = 1;
+class Foo {
+  method1(foo2 = 2) {
+  }
+  method2(foo2 = 3) {
+  }
+}
+__decorateClass([
+  __decorateParam(0, dec(foo))
+], Foo.prototype, "method1", 1);
+__decorateClass([
+  __decorateParam(0, dec(() => foo))
+], Foo.prototype, "method2", 1);
+class Bar {
+}
+Bar.x = (_a = class {
+}, _a.y = () => {
+  let bar = 1;
+  let Baz = class {
+    method1() {
+    }
+    method2() {
+    }
+    method3(bar2) {
+    }
+    method4(bar2) {
+    }
+  };
+  __decorateClass([
+    dec(bar)
+  ], Baz.prototype, "method1", 1);
+  __decorateClass([
+    dec(() => bar)
+  ], Baz.prototype, "method2", 1);
+  __decorateClass([
+    __decorateParam(0, dec(() => bar))
+  ], Baz.prototype, "method3", 1);
+  __decorateClass([
+    __decorateParam(0, dec(() => bar))
+  ], Baz.prototype, "method4", 1);
+  Baz = __decorateClass([
+    dec(bar),
+    dec(() => bar)
+  ], Baz);
+  return Baz;
+}, _a);
+
 ================================================================================
 TestTypeScriptDecorators
 ---------- /out.js ----------
@@ -1074,7 +1125,7 @@ var k_default = class {
   }
 };
 __decorateClass([
-  __decorateParam(0, x2(() => 0)),
+  __decorateParam(0, x(() => 0)),
   __decorateParam(0, y(() => 1))
 ], k_default.prototype, "foo", 1);