fix #3761: this in field and accessor decorators

evanw · May 12, 2024 · f82e0ad · f82e0ad
1 parent 46ea596
commit f82e0ad
Show file tree

Hide file tree

Showing 7 changed files with 311 additions and 235 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,16 @@
 
 ## Unreleased
 
+* Correct `this` in field and accessor decorators ([#3761](https://github.com/evanw/esbuild/issues/3761))
+
+    This release changes the value of `this` in initializers for class field and accessor decorators from the module-level `this` value to the appropriate `this` value for the decorated element (either the class or the instance). It was previously incorrect due to lack of test coverage. Here's an example of a decorator that doesn't work without this change:
+
+    ```js
+    const dec = () => function() { this.bar = true }
+    class Foo { @dec static foo }
+    console.log(Foo.bar) // Should be "true"
+    ```
+
 * Allow `es2023` as a target environment ([#3762](https://github.com/evanw/esbuild/issues/3762))
 
     TypeScript recently [added `es2023`](https://github.com/microsoft/TypeScript/pull/58140) as a compilation target, so esbuild now supports this too. There is no difference between a target of `es2022` and `es2023` as far as esbuild is concerned since the 2023 edition of JavaScript doesn't introduce any new syntax features.

diff --git a/internal/js_parser/js_parser_lower_class.go b/internal/js_parser/js_parser_lower_class.go
@@ -850,9 +850,16 @@ func (ctx *lowerClassContext) lowerField(
 
 		// Optionally call registered decorator initializers
 		if initializerIndex != -1 {
+			var value js_ast.Expr
+			if prop.Flags.Has(js_ast.PropertyIsStatic) {
+				value = ctx.nameFunc()
+			} else {
+				value = js_ast.Expr{Loc: loc, Data: js_ast.EThisShared}
+			}
 			args := []js_ast.Expr{
 				{Loc: loc, Data: &js_ast.EIdentifier{Ref: ctx.decoratorContextRef}},
 				{Loc: loc, Data: &js_ast.ENumber{Value: float64((3 + 2*initializerIndex) << 1)}},
+				value,
 			}
 			if _, ok := init.Data.(*js_ast.EUndefined); !ok {
 				args = append(args, init)
@@ -1750,9 +1757,16 @@ func (ctx *lowerClassContext) processProperties(p *parser, classLoweringInfo cla
 
 				// Optionally call registered decorator initializers
 				if initializerIndex != -1 {
+					var value js_ast.Expr
+					if prop.Flags.Has(js_ast.PropertyIsStatic) {
+						value = ctx.nameFunc()
+					} else {
+						value = js_ast.Expr{Loc: loc, Data: js_ast.EThisShared}
+					}
 					args := []js_ast.Expr{
 						{Loc: loc, Data: &js_ast.EIdentifier{Ref: ctx.decoratorContextRef}},
 						{Loc: loc, Data: &js_ast.ENumber{Value: float64((3 + 2*initializerIndex) << 1)}},
+						value,
 					}
 					if _, ok := init.Data.(*js_ast.EUndefined); !ok {
 						args = append(args, init)

diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go
@@ -2087,7 +2087,7 @@ _init = [, , ,];
 _x_dec = [dec];
 class Foo {
   constructor() {
-    __publicField(this, "x", __runInitializers(_init, 6)), __runInitializers(_init, 9, this);
+    __publicField(this, "x", __runInitializers(_init, 6, this)), __runInitializers(_init, 9, this);
   }
 }
 __decorateElement(_init, 5, "x", _x_dec, Foo);
@@ -2111,7 +2111,7 @@ _init = [, , ,];
 _x_dec = [dec];
 class Foo {
   constructor() {
-    __privateAdd(this, _x, __runInitializers(_init, 6)), __runInitializers(_init, 9, this);
+    __privateAdd(this, _x, __runInitializers(_init, 6, this)), __runInitializers(_init, 9, this);
   }
 }
 _x = new WeakMap();
@@ -2124,7 +2124,7 @@ _x_dec = [dec];
 class Foo {
 }
 __decorateElement(_init, 13, "x", _x_dec, Foo);
-__publicField(Foo, "x", __runInitializers(_init, 6)), __runInitializers(_init, 9, Foo);
+__publicField(Foo, "x", __runInitializers(_init, 6, Foo)), __runInitializers(_init, 9, Foo);
 `)
 	expectPrintedWithUnsupportedFeatures(t, compat.Decorators, "class Foo { @dec static x() {} }",
 		`var _x_dec, _init;
@@ -2145,7 +2145,7 @@ class Foo {
 }
 _x = new WeakMap();
 __decorateElement(_init, 12, "x", _x_dec, Foo, _x);
-__privateAdd(Foo, _x, __runInitializers(_init, 6)), __runInitializers(_init, 9, Foo);
+__privateAdd(Foo, _x, __runInitializers(_init, 6, Foo)), __runInitializers(_init, 9, Foo);
 `)
 
 	// Check ASI for "abstract"

diff --git a/internal/js_parser/ts_parser_test.go b/internal/js_parser/ts_parser_test.go
@@ -2132,7 +2132,7 @@ _init = [, , ,];
 _x_dec = [dec];
 class Foo {
   constructor() {
-    __publicField(this, "x", __runInitializers(_init, 6)), __runInitializers(_init, 9, this);
+    __publicField(this, "x", __runInitializers(_init, 6, this)), __runInitializers(_init, 9, this);
   }
 }
 __decorateElement(_init, 5, "x", _x_dec, Foo);
@@ -2156,7 +2156,7 @@ _init = [, , ,];
 _x_dec = [dec];
 class Foo {
   constructor() {
-    __privateAdd(this, _x, __runInitializers(_init, 6)), __runInitializers(_init, 9, this);
+    __privateAdd(this, _x, __runInitializers(_init, 6, this)), __runInitializers(_init, 9, this);
   }
 }
 _x = new WeakMap();
@@ -2169,7 +2169,7 @@ _x_dec = [dec];
 class Foo {
 }
 __decorateElement(_init, 13, "x", _x_dec, Foo);
-__publicField(Foo, "x", __runInitializers(_init, 6)), __runInitializers(_init, 9, Foo);
+__publicField(Foo, "x", __runInitializers(_init, 6, Foo)), __runInitializers(_init, 9, Foo);
 `)
 	expectPrintedWithUnsupportedFeaturesTS(t, compat.Decorators, "class Foo { @dec static x() {} }",
 		`var _x_dec, _init;
@@ -2190,7 +2190,7 @@ class Foo {
 }
 _x = new WeakMap();
 __decorateElement(_init, 12, "x", _x_dec, Foo, _x);
-__privateAdd(Foo, _x, __runInitializers(_init, 6)), __runInitializers(_init, 9, Foo);
+__privateAdd(Foo, _x, __runInitializers(_init, 6, Foo)), __runInitializers(_init, 9, Foo);
 `)
 
 	// Check ASI for "abstract"

diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go
@@ -265,8 +265,8 @@ func Source(unsupportedJSFeatures compat.JSFeature) logger.Source {
 		var __expectFn = fn => fn !== void 0 && typeof fn !== 'function' ? __typeError('Function expected') : fn
 		var __decoratorContext = (kind, name, done, fns) => ({ kind: __decoratorStrings[kind], name, addInitializer: fn =>
 			done._ ? __typeError('Already initialized') : fns.push(__expectFn(fn || null)), })
-		export var __runInitializers = (array, flags, value) => {
-			for (var i = 0, fns = array[flags >> 1], n = fns && fns.length; i < n; i++) flags & 1 ? fns[i].call(value) : value = (0, fns[i])(value)
+		export var __runInitializers = (array, flags, self, value) => {
+			for (var i = 0, fns = array[flags >> 1], n = fns && fns.length; i < n; i++) flags & 1 ? fns[i].call(self) : value = fns[i].call(self, value)
 			return value
 		}
 		export var __decorateElement = (array, flags, name, decorators, target, extra) => {