From 66b7c6d174e0fbe693c7d7545c7430ad20d47c39 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Wed, 15 May 2024 14:46:41 -0400 Subject: [PATCH] fix #3768: bundled decorators in derived classes --- CHANGELOG.md | 13 ++ internal/bundler_tests/bundler_lower_test.go | 28 +++ .../snapshots/snapshots_lower.txt | 159 ++++++++++++++++++ internal/js_parser/js_parser_lower_class.go | 7 +- scripts/end-to-end-tests.js | 42 +++++ 5 files changed, 248 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b2d719c90e..fcb0f0bef59 100644 --- 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 index 0e97439bfe8..c5cf8484460 100644 --- 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 index cbdd555eeea..6416316fb8b 100644 --- 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 index 7e12cb7805c..68fa323dcc2 100644 --- 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 index 8766064738b..c2370ba65cc 100644 --- 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