fix #3781: add metadata to all decorated classes

evanw · Jun 9, 2024 · b93a2a9 · b93a2a9
1 parent 953dae9
commit b93a2a9
Show file tree

Hide file tree

Showing 8 changed files with 247 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Fix `Symbol.metadata` on classes without a class decorator ([#3781](https://github.com/evanw/esbuild/issues/3781))
+
+    This release fixes a bug with esbuild's support for the [decorator metadata proposal](https://github.com/tc39/proposal-decorator-metadata). Previously esbuild only added the `Symbol.metadata` property to decorated classes if there was a decorator on the class element itself. However, the proposal says that the `Symbol.metadata` property should be present on all classes that have any decorators at all, not just those with a decorator on the class element itself.
+
 * Allow unknown import attributes to be used with the `copy` loader ([#3792](https://github.com/evanw/esbuild/issues/3792))
 
     Import attributes (the `with` keyword on `import` statements) are allowed to alter how that path is loaded. For example, esbuild cannot assume that it knows how to load `./bagel.js` as type `bagel`:

diff --git a/internal/bundler_tests/snapshots/snapshots_lower.txt b/internal/bundler_tests/snapshots/snapshots_lower.txt
@@ -633,6 +633,7 @@ var _Foo = class _Foo {
 _init = __decoratorStart(null);
 _foo = new WeakMap();
 __decorateElement(_init, 4, "foo", _foo_dec, _Foo, _foo);
+__decoratorMetadata(_init, _Foo);
 var Foo = _Foo;
 
 ---------- /out/base-instance-field.js ----------
@@ -646,6 +647,7 @@ var _Foo = class _Foo {
 };
 _init = __decoratorStart(null);
 __decorateElement(_init, 5, "foo", _foo_dec, _Foo);
+__decoratorMetadata(_init, _Foo);
 var Foo = _Foo;
 
 ---------- /out/base-instance-method.js ----------
@@ -662,6 +664,7 @@ var _Foo = class _Foo {
 };
 _init = __decoratorStart(null);
 __decorateElement(_init, 1, "foo", _foo_dec, _Foo);
+__decoratorMetadata(_init, _Foo);
 var Foo = _Foo;
 
 ---------- /out/base-static-accessor.js ----------
@@ -673,6 +676,7 @@ var _Foo = class _Foo {
 _init = __decoratorStart(null);
 _foo = new WeakMap();
 __decorateElement(_init, 12, "foo", _foo_dec, _Foo, _foo);
+__decoratorMetadata(_init, _Foo);
 __privateAdd(_Foo, _foo, __runInitializers(_init, 8, _Foo, _Foo)), __runInitializers(_init, 11, _Foo);
 var Foo = _Foo;
 
@@ -684,6 +688,7 @@ var _Foo = class _Foo {
 };
 _init = __decoratorStart(null);
 __decorateElement(_init, 13, "foo", _foo_dec, _Foo);
+__decoratorMetadata(_init, _Foo);
 __publicField(_Foo, "foo", __runInitializers(_init, 8, _Foo, _Foo)), __runInitializers(_init, 11, _Foo);
 var Foo = _Foo;
 
@@ -698,6 +703,7 @@ var _Foo = class _Foo {
 };
 _init = __decoratorStart(null);
 __decorateElement(_init, 9, "foo", _foo_dec, _Foo);
+__decoratorMetadata(_init, _Foo);
 __runInitializers(_init, 3, _Foo);
 var Foo = _Foo;
 
@@ -713,6 +719,7 @@ var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
 _init = __decoratorStart(_a);
 _foo = new WeakMap();
 __decorateElement(_init, 4, "foo", _foo_dec, _Foo, _foo);
+__decoratorMetadata(_init, _Foo);
 var Foo = _Foo;
 
 ---------- /out/derived-instance-field.js ----------
@@ -726,6 +733,7 @@ var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
 };
 _init = __decoratorStart(_a);
 __decorateElement(_init, 5, "foo", _foo_dec, _Foo);
+__decoratorMetadata(_init, _Foo);
 var Foo = _Foo;
 
 ---------- /out/derived-instance-method.js ----------
@@ -742,6 +750,7 @@ var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
 };
 _init = __decoratorStart(_a);
 __decorateElement(_init, 1, "foo", _foo_dec, _Foo);
+__decoratorMetadata(_init, _Foo);
 var Foo = _Foo;
 
 ---------- /out/derived-static-accessor.js ----------
@@ -752,6 +761,7 @@ var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
 _init = __decoratorStart(_a);
 _foo = new WeakMap();
 __decorateElement(_init, 12, "foo", _foo_dec, _Foo, _foo);
+__decoratorMetadata(_init, _Foo);
 __privateAdd(_Foo, _foo, __runInitializers(_init, 8, _Foo, _Foo)), __runInitializers(_init, 11, _Foo);
 var Foo = _Foo;
 
@@ -762,6 +772,7 @@ var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
 };
 _init = __decoratorStart(_a);
 __decorateElement(_init, 13, "foo", _foo_dec, _Foo);
+__decoratorMetadata(_init, _Foo);
 __publicField(_Foo, "foo", __runInitializers(_init, 8, _Foo, _Foo)), __runInitializers(_init, 11, _Foo);
 var Foo = _Foo;
 
@@ -775,6 +786,7 @@ var _Foo = class _Foo extends (_a = Bar, _foo_dec = [dec], _a) {
 };
 _init = __decoratorStart(_a);
 __decorateElement(_init, 9, "foo", _foo_dec, _Foo);
+__decoratorMetadata(_init, _Foo);
 __runInitializers(_init, 3, _Foo);
 var Foo = _Foo;
 

diff --git a/internal/js_parser/js_parser_lower_class.go b/internal/js_parser/js_parser_lower_class.go
@@ -2083,9 +2083,9 @@ func (ctx *lowerClassContext) finishAndGenerateCode(p *parser, result visitClass
 		classDecorators = ctx.decoratorClassDecorators
 	}
 
-	// Handle JavaScript decorators on the class itself
 	var decorateClassExpr js_ast.Expr
 	if classDecorators.Data != nil {
+		// Handle JavaScript decorators on the class itself
 		if ctx.decoratorContextRef == ast.InvalidRef {
 			ctx.decoratorContextRef = p.generateTempRef(tempRefNeedsDeclare, "_init")
 		}
@@ -2098,6 +2098,12 @@ func (ctx *lowerClassContext) finishAndGenerateCode(p *parser, result visitClass
 		})
 		p.recordUsage(ctx.decoratorContextRef)
 		decorateClassExpr = js_ast.Assign(ctx.nameFunc(), decorateClassExpr)
+	} else if ctx.decoratorContextRef != ast.InvalidRef {
+		// Decorator metadata is present if there are any decorators on the class at all
+		decorateClassExpr = p.callRuntime(ctx.classLoc, "__decoratorMetadata", []js_ast.Expr{
+			{Loc: ctx.classLoc, Data: &js_ast.EIdentifier{Ref: ctx.decoratorContextRef}},
+			ctx.nameFunc(),
+		})
 	}
 
 	// If this is true, we have removed some code from the class body that could
@@ -2203,7 +2209,7 @@ func (ctx *lowerClassContext) finishAndGenerateCode(p *parser, result visitClass
 	suffixExprs = append(suffixExprs, ctx.staticExperimentalDecorators...)
 
 	// For each element initializer of classExtraInitializers
-	if decorateClassExpr.Data != nil {
+	if classDecorators.Data != nil {
 		suffixExprs = append(suffixExprs, p.callRuntime(ctx.classLoc, "__runInitializers", []js_ast.Expr{
 			{Loc: ctx.classLoc, Data: &js_ast.EIdentifier{Ref: ctx.decoratorContextRef}},
 			{Loc: ctx.classLoc, Data: &js_ast.ENumber{Value: (0 << 1) | 1}},

diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go
@@ -2091,6 +2091,7 @@ class Foo {
 }
 _init = __decoratorStart(null);
 __decorateElement(_init, 5, "x", _x_dec, Foo);
+__decoratorMetadata(_init, Foo);
 `)
 	expectPrintedWithUnsupportedFeatures(t, compat.Decorators, "class Foo { @dec x() {} }",
 		`var _x_dec, _init;
@@ -2104,6 +2105,7 @@ class Foo {
 }
 _init = __decoratorStart(null);
 __decorateElement(_init, 1, "x", _x_dec, Foo);
+__decoratorMetadata(_init, Foo);
 `)
 	expectPrintedWithUnsupportedFeatures(t, compat.Decorators, "class Foo { @dec accessor x }",
 		`var _x_dec, _init, _x;
@@ -2116,6 +2118,7 @@ class Foo {
 _init = __decoratorStart(null);
 _x = new WeakMap();
 __decorateElement(_init, 4, "x", _x_dec, Foo, _x);
+__decoratorMetadata(_init, Foo);
 `)
 	expectPrintedWithUnsupportedFeatures(t, compat.Decorators, "class Foo { @dec static x }",
 		`var _x_dec, _init;
@@ -2124,6 +2127,7 @@ class Foo {
 }
 _init = __decoratorStart(null);
 __decorateElement(_init, 13, "x", _x_dec, Foo);
+__decoratorMetadata(_init, Foo);
 __publicField(Foo, "x", __runInitializers(_init, 8, Foo)), __runInitializers(_init, 11, Foo);
 `)
 	expectPrintedWithUnsupportedFeatures(t, compat.Decorators, "class Foo { @dec static x() {} }",
@@ -2135,6 +2139,7 @@ class Foo {
 }
 _init = __decoratorStart(null);
 __decorateElement(_init, 9, "x", _x_dec, Foo);
+__decoratorMetadata(_init, Foo);
 __runInitializers(_init, 3, Foo);
 `)
 	expectPrintedWithUnsupportedFeatures(t, compat.Decorators, "class Foo { @dec static accessor x }",
@@ -2145,6 +2150,7 @@ class Foo {
 _init = __decoratorStart(null);
 _x = new WeakMap();
 __decorateElement(_init, 12, "x", _x_dec, Foo, _x);
+__decoratorMetadata(_init, Foo);
 __privateAdd(Foo, _x, __runInitializers(_init, 8, Foo)), __runInitializers(_init, 11, Foo);
 `)
 

diff --git a/internal/js_parser/ts_parser_test.go b/internal/js_parser/ts_parser_test.go
@@ -2136,6 +2136,7 @@ class Foo {
 }
 _init = __decoratorStart(null);
 __decorateElement(_init, 5, "x", _x_dec, Foo);
+__decoratorMetadata(_init, Foo);
 `)
 	expectPrintedWithUnsupportedFeaturesTS(t, compat.Decorators, "class Foo { @dec x() {} }",
 		`var _x_dec, _init;
@@ -2149,6 +2150,7 @@ class Foo {
 }
 _init = __decoratorStart(null);
 __decorateElement(_init, 1, "x", _x_dec, Foo);
+__decoratorMetadata(_init, Foo);
 `)
 	expectPrintedWithUnsupportedFeaturesTS(t, compat.Decorators, "class Foo { @dec accessor x }",
 		`var _x_dec, _init, _x;
@@ -2161,6 +2163,7 @@ class Foo {
 _init = __decoratorStart(null);
 _x = new WeakMap();
 __decorateElement(_init, 4, "x", _x_dec, Foo, _x);
+__decoratorMetadata(_init, Foo);
 `)
 	expectPrintedWithUnsupportedFeaturesTS(t, compat.Decorators, "class Foo { @dec static x }",
 		`var _x_dec, _init;
@@ -2169,6 +2172,7 @@ class Foo {
 }
 _init = __decoratorStart(null);
 __decorateElement(_init, 13, "x", _x_dec, Foo);
+__decoratorMetadata(_init, Foo);
 __publicField(Foo, "x", __runInitializers(_init, 8, Foo)), __runInitializers(_init, 11, Foo);
 `)
 	expectPrintedWithUnsupportedFeaturesTS(t, compat.Decorators, "class Foo { @dec static x() {} }",
@@ -2180,6 +2184,7 @@ class Foo {
 }
 _init = __decoratorStart(null);
 __decorateElement(_init, 9, "x", _x_dec, Foo);
+__decoratorMetadata(_init, Foo);
 __runInitializers(_init, 3, Foo);
 `)
 	expectPrintedWithUnsupportedFeaturesTS(t, compat.Decorators, "class Foo { @dec static accessor x }",
@@ -2190,6 +2195,7 @@ class Foo {
 _init = __decoratorStart(null);
 _x = new WeakMap();
 __decorateElement(_init, 12, "x", _x_dec, Foo, _x);
+__decoratorMetadata(_init, Foo);
 __privateAdd(Foo, _x, __runInitializers(_init, 8, Foo)), __runInitializers(_init, 11, Foo);
 `)
 

diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go
@@ -266,6 +266,7 @@ 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, metadata, fns) => ({ kind: __decoratorStrings[kind], name, metadata, addInitializer: fn =>
 			done._ ? __typeError('Already initialized') : fns.push(__expectFn(fn || null)) })
+		export var __decoratorMetadata = (array, target) => __defNormalProp(target, __knownSymbol('metadata'), array[3])
 		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
@@ -306,7 +307,7 @@ func Source(unsupportedJSFeatures compat.JSFeature) logger.Source {
 				else __expectFn(fn = it.get) && (desc.get = fn), __expectFn(fn = it.set) && (desc.set = fn), __expectFn(fn = it.init) && initializers.unshift(fn)
 			}
 
-			return k || (target[__knownSymbol('metadata')] = array[3]),
+			return k || __decoratorMetadata(array, target),
 				desc && __defProp(target, name, desc),
 				p ? k ^ 4 ? extra : desc : target
 		}