From 3af5d3b4edf703fba24a15653e4d3926b2786793 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sat, 17 Jun 2023 19:10:41 -0400 Subject: [PATCH] fix #3177: experimental decorators + mangle props --- CHANGELOG.md | 4 + internal/bundler_tests/bundler_ts_test.go | 173 +++++++++++++++ .../bundler_tests/snapshots/snapshots_ts.txt | 200 ++++++++++++++++++ internal/js_parser/js_parser_lower.go | 36 ++-- scripts/end-to-end-tests.js | 58 +++++ 5 files changed, 457 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e2602a078e..d2bfb1ed62d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ new Foo().foo = Foo.bar; ``` +* Fix TypeScript experimental decorators combined with `--mangle-props` ([#3177](https://github.com/evanw/esbuild/issues/3177)) + + Previously using TypeScript experimental decorators combined with the `--mangle-props` setting could result in a crash, as the experimental decorator transform was not expecting a mangled property as a class member. This release fixes the crash so you can now combine both of these features together safely. + ## 0.18.4 * Bundling no longer unnecessarily transforms class syntax ([#1360](https://github.com/evanw/esbuild/issues/1360), [#1328](https://github.com/evanw/esbuild/issues/1328), [#1524](https://github.com/evanw/esbuild/issues/1524), [#2416](https://github.com/evanw/esbuild/issues/2416)) diff --git a/internal/bundler_tests/bundler_ts_test.go b/internal/bundler_tests/bundler_ts_test.go index 4f6863debf8..4d90d1948d3 100644 --- a/internal/bundler_tests/bundler_ts_test.go +++ b/internal/bundler_tests/bundler_ts_test.go @@ -1,6 +1,7 @@ package bundler_tests import ( + "regexp" "testing" "github.com/evanw/esbuild/internal/compat" @@ -2470,3 +2471,175 @@ func TestTSPreferJSOverTSInsideNodeModules(t *testing.T) { }, }) } + +func TestTSExperimentalDecoratorsManglePropsDefineSemantics(t *testing.T) { + ts_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.ts": ` + class Foo { + @dec(1) prop1 = null + @dec(2) prop2_ = null + @dec(3) ['prop3'] = null + @dec(4) ['prop4_'] = null + @dec(5) [/* @__KEY__ */ 'prop5'] = null + @dec(6) [/* @__KEY__ */ 'prop6_'] = null + } + `, + "/tsconfig.json": `{ + "compilerOptions": { + "experimentalDecorators": true, + "useDefineForClassFields": true, + }, + }`, + }, + entryPaths: []string{"/entry.ts"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + MangleProps: regexp.MustCompile("_$"), + }, + }) +} + +func TestTSExperimentalDecoratorsManglePropsAssignSemantics(t *testing.T) { + ts_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.ts": ` + class Foo { + @dec(1) prop1 = null + @dec(2) prop2_ = null + @dec(3) ['prop3'] = null + @dec(4) ['prop4_'] = null + @dec(5) [/* @__KEY__ */ 'prop5'] = null + @dec(6) [/* @__KEY__ */ 'prop6_'] = null + } + `, + "/tsconfig.json": `{ + "compilerOptions": { + "experimentalDecorators": true, + "useDefineForClassFields": false, + }, + }`, + }, + entryPaths: []string{"/entry.ts"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + MangleProps: regexp.MustCompile("_$"), + }, + }) +} + +func TestTSExperimentalDecoratorsManglePropsMethods(t *testing.T) { + ts_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.ts": ` + class Foo { + @dec(1) prop1() {} + @dec(2) prop2_() {} + @dec(3) ['prop3']() {} + @dec(4) ['prop4_']() {} + @dec(5) [/* @__KEY__ */ 'prop5']() {} + @dec(6) [/* @__KEY__ */ 'prop6_']() {} + } + `, + "/tsconfig.json": `{ + "compilerOptions": { + "experimentalDecorators": true, + }, + }`, + }, + entryPaths: []string{"/entry.ts"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + MangleProps: regexp.MustCompile("_$"), + }, + }) +} + +func TestTSExperimentalDecoratorsManglePropsStaticDefineSemantics(t *testing.T) { + ts_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.ts": ` + class Foo { + @dec(1) static prop1 = null + @dec(2) static prop2_ = null + @dec(3) static ['prop3'] = null + @dec(4) static ['prop4_'] = null + @dec(5) static [/* @__KEY__ */ 'prop5'] = null + @dec(6) static [/* @__KEY__ */ 'prop6_'] = null + } + `, + "/tsconfig.json": `{ + "compilerOptions": { + "experimentalDecorators": true, + "useDefineForClassFields": true, + }, + }`, + }, + entryPaths: []string{"/entry.ts"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + MangleProps: regexp.MustCompile("_$"), + }, + }) +} + +func TestTSExperimentalDecoratorsManglePropsStaticAssignSemantics(t *testing.T) { + ts_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.ts": ` + class Foo { + @dec(1) static prop1 = null + @dec(2) static prop2_ = null + @dec(3) static ['prop3'] = null + @dec(4) static ['prop4_'] = null + @dec(5) static [/* @__KEY__ */ 'prop5'] = null + @dec(6) static [/* @__KEY__ */ 'prop6_'] = null + } + `, + "/tsconfig.json": `{ + "compilerOptions": { + "experimentalDecorators": true, + "useDefineForClassFields": false, + }, + }`, + }, + entryPaths: []string{"/entry.ts"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + MangleProps: regexp.MustCompile("_$"), + }, + }) +} + +func TestTSExperimentalDecoratorsManglePropsStaticMethods(t *testing.T) { + ts_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.ts": ` + class Foo { + @dec(1) static prop1() {} + @dec(2) static prop2_() {} + @dec(3) static ['prop3']() {} + @dec(4) static ['prop4_']() {} + @dec(5) static [/* @__KEY__ */ 'prop5']() {} + @dec(6) static [/* @__KEY__ */ 'prop6_']() {} + } + `, + "/tsconfig.json": `{ + "compilerOptions": { + "experimentalDecorators": true, + }, + }`, + }, + entryPaths: []string{"/entry.ts"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + MangleProps: regexp.MustCompile("_$"), + }, + }) +} diff --git a/internal/bundler_tests/snapshots/snapshots_ts.txt b/internal/bundler_tests/snapshots/snapshots_ts.txt index b9b5d0a2f9d..9ab2e35a944 100644 --- a/internal/bundler_tests/snapshots/snapshots_ts.txt +++ b/internal/bundler_tests/snapshots/snapshots_ts.txt @@ -895,6 +895,206 @@ export { Foo }; +================================================================================ +TestTSExperimentalDecoratorsManglePropsAssignSemantics +---------- /out.js ---------- +// entry.ts +var Foo = class { + constructor() { + this.prop1 = null; + this.a = null; + this["prop3"] = null; + this["prop4_"] = null; + this[/* @__KEY__ */ "prop5"] = null; + this.b = null; + } +}; +__decorateClass([ + dec(1) +], Foo.prototype, "prop1", 2); +__decorateClass([ + dec(2) +], Foo.prototype, "a", 2); +__decorateClass([ + dec(3) +], Foo.prototype, "prop3", 2); +__decorateClass([ + dec(4) +], Foo.prototype, "prop4_", 2); +__decorateClass([ + dec(5) +], Foo.prototype, /* @__KEY__ */ "prop5", 2); +__decorateClass([ + dec(6) +], Foo.prototype, /* @__KEY__ */ "b", 2); + +================================================================================ +TestTSExperimentalDecoratorsManglePropsDefineSemantics +---------- /out.js ---------- +// entry.ts +var Foo = class { + prop1 = null; + a = null; + ["prop3"] = null; + ["prop4_"] = null; + [/* @__KEY__ */ "prop5"] = null; + [/* @__KEY__ */ "b"] = null; +}; +__decorateClass([ + dec(1) +], Foo.prototype, "prop1", 2); +__decorateClass([ + dec(2) +], Foo.prototype, "a", 2); +__decorateClass([ + dec(3) +], Foo.prototype, "prop3", 2); +__decorateClass([ + dec(4) +], Foo.prototype, "prop4_", 2); +__decorateClass([ + dec(5) +], Foo.prototype, /* @__KEY__ */ "prop5", 2); +__decorateClass([ + dec(6) +], Foo.prototype, /* @__KEY__ */ "b", 2); + +================================================================================ +TestTSExperimentalDecoratorsManglePropsMethods +---------- /out.js ---------- +// entry.ts +var Foo = class { + prop1() { + } + a() { + } + ["prop3"]() { + } + ["prop4_"]() { + } + [/* @__KEY__ */ "prop5"]() { + } + [/* @__KEY__ */ "b"]() { + } +}; +__decorateClass([ + dec(1) +], Foo.prototype, "prop1", 1); +__decorateClass([ + dec(2) +], Foo.prototype, "a", 1); +__decorateClass([ + dec(3) +], Foo.prototype, "prop3", 1); +__decorateClass([ + dec(4) +], Foo.prototype, "prop4_", 1); +__decorateClass([ + dec(5) +], Foo.prototype, /* @__KEY__ */ "prop5", 1); +__decorateClass([ + dec(6) +], Foo.prototype, /* @__KEY__ */ "b", 1); + +================================================================================ +TestTSExperimentalDecoratorsManglePropsStaticAssignSemantics +---------- /out.js ---------- +// entry.ts +var Foo = class { +}; +Foo.prop1 = null; +Foo.a = null; +Foo["prop3"] = null; +Foo["prop4_"] = null; +Foo[/* @__KEY__ */ "prop5"] = null; +Foo.b = null; +__decorateClass([ + dec(1) +], Foo, "prop1", 2); +__decorateClass([ + dec(2) +], Foo, "a", 2); +__decorateClass([ + dec(3) +], Foo, "prop3", 2); +__decorateClass([ + dec(4) +], Foo, "prop4_", 2); +__decorateClass([ + dec(5) +], Foo, /* @__KEY__ */ "prop5", 2); +__decorateClass([ + dec(6) +], Foo, /* @__KEY__ */ "b", 2); + +================================================================================ +TestTSExperimentalDecoratorsManglePropsStaticDefineSemantics +---------- /out.js ---------- +// entry.ts +var Foo = class { + static prop1 = null; + static a = null; + static ["prop3"] = null; + static ["prop4_"] = null; + static [/* @__KEY__ */ "prop5"] = null; + static [/* @__KEY__ */ "b"] = null; +}; +__decorateClass([ + dec(1) +], Foo, "prop1", 2); +__decorateClass([ + dec(2) +], Foo, "a", 2); +__decorateClass([ + dec(3) +], Foo, "prop3", 2); +__decorateClass([ + dec(4) +], Foo, "prop4_", 2); +__decorateClass([ + dec(5) +], Foo, /* @__KEY__ */ "prop5", 2); +__decorateClass([ + dec(6) +], Foo, /* @__KEY__ */ "b", 2); + +================================================================================ +TestTSExperimentalDecoratorsManglePropsStaticMethods +---------- /out.js ---------- +// entry.ts +var Foo = class { + static prop1() { + } + static a() { + } + static ["prop3"]() { + } + static ["prop4_"]() { + } + static [/* @__KEY__ */ "prop5"]() { + } + static [/* @__KEY__ */ "b"]() { + } +}; +__decorateClass([ + dec(1) +], Foo, "prop1", 1); +__decorateClass([ + dec(2) +], Foo, "a", 1); +__decorateClass([ + dec(3) +], Foo, "prop3", 1); +__decorateClass([ + dec(4) +], Foo, "prop4_", 1); +__decorateClass([ + dec(5) +], Foo, /* @__KEY__ */ "prop5", 1); +__decorateClass([ + dec(6) +], Foo, /* @__KEY__ */ "b", 1); + ================================================================================ TestTSExportDefaultTypeIssue316 ---------- /out.js ---------- diff --git a/internal/js_parser/js_parser_lower.go b/internal/js_parser/js_parser_lower.go index 198e891dc73..ec5926f607c 100644 --- a/internal/js_parser/js_parser_lower.go +++ b/internal/js_parser/js_parser_lower.go @@ -2405,15 +2405,17 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas // Make sure the order of computed property keys doesn't change. These // expressions have side effects and must be evaluated in order. keyExprNoSideEffects := prop.Key - if prop.Flags.Has(js_ast.PropertyIsComputed) && - (len(prop.Decorators) > 0 || mustLowerField || computedPropertyCache.Data != nil) { + if prop.Flags.Has(js_ast.PropertyIsComputed) && (len(prop.Decorators) > 0 || mustLowerField || computedPropertyCache.Data != nil) { needsKey := true if len(prop.Decorators) == 0 && (prop.Flags.Has(js_ast.PropertyIsMethod) || shouldOmitFieldInitializer || !mustLowerField) { needsKey = false } - // Assume all non-string computed keys have important side effects - if _, ok := prop.Key.Data.(*js_ast.EString); !ok { + // Assume all non-literal computed keys have important side effects + switch prop.Key.Data.(type) { + case *js_ast.EString, *js_ast.EMangledProp, *js_ast.ENumber: + // These have no side effects + default: if !needsKey { // Just evaluate the key for its side effects computedPropertyCache = js_ast.JoinWithComma(computedPropertyCache, prop.Key) @@ -2426,14 +2428,14 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas prop.Key = js_ast.Expr{Loc: prop.Key.Loc, Data: &js_ast.EIdentifier{Ref: ref}} keyExprNoSideEffects = prop.Key } - } - // If this is a computed method, the property value will be used - // immediately. In this case we inline all computed properties so far to - // make sure all computed properties before this one are evaluated first. - if !mustLowerField { - prop.Key = computedPropertyCache - computedPropertyCache = js_ast.Expr{} + // If this is a computed method, the property value will be used + // immediately. In this case we inline all computed properties so far to + // make sure all computed properties before this one are evaluated first. + if !mustLowerField { + prop.Key = computedPropertyCache + computedPropertyCache = js_ast.Expr{} + } } } @@ -2447,11 +2449,17 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas var descriptorKey js_ast.Expr switch k := keyExprNoSideEffects.Data.(type) { case *js_ast.ENumber: - descriptorKey = js_ast.Expr{Loc: loc, Data: &js_ast.ENumber{Value: k.Value}} + clone := *k + descriptorKey = js_ast.Expr{Loc: loc, Data: &clone} case *js_ast.EString: - descriptorKey = js_ast.Expr{Loc: loc, Data: &js_ast.EString{Value: k.Value}} + clone := *k + descriptorKey = js_ast.Expr{Loc: loc, Data: &clone} case *js_ast.EIdentifier: - descriptorKey = js_ast.Expr{Loc: loc, Data: &js_ast.EIdentifier{Ref: k.Ref}} + clone := *k + descriptorKey = js_ast.Expr{Loc: loc, Data: &clone} + case *js_ast.EMangledProp: + clone := *k + descriptorKey = js_ast.Expr{Loc: loc, Data: &clone} default: panic("Internal error") } diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index ba3a363c9a2..5ba98572b41 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -4812,6 +4812,64 @@ for (let flags of [[], ['--target=es6'], ['--bundle'], ['--bundle', '--target=es if (!staticMethod) throw 'fail: staticMethod' `, }), + + // https://github.com/evanw/esbuild/issues/3177 + test(['in.ts', '--outfile=node.js', '--mangle-props=_'].concat(flags), { + 'in.ts': ` + const props: Record = {} + const dec = (n: number) => (_: any, prop: string): void => { + props[n] = prop + } + class Foo { + @dec(1) prop1: any + @dec(2) prop2_: any + @dec(3) ['prop3']: any + @dec(4) ['prop4_']: any + @dec(5) [/* @__KEY__ */ 'prop5']: any + @dec(6) [/* @__KEY__ */ 'prop6_']: any + } + if (props[1] !== 'prop1') throw 'fail 1: ' + props[1] + if (props[2] !== /* @__KEY__ */ 'prop2_') throw 'fail 2: ' + props[2] + if (props[3] !== 'prop3') throw 'fail 3: ' + props[3] + if (props[4] !== 'prop4_') throw 'fail 4: ' + props[4] + if (props[5] !== 'prop5') throw 'fail 5: ' + props[5] + if (props[6] !== /* @__KEY__ */ 'prop6_') throw 'fail 6: ' + props[6] + `, + 'tsconfig.json': `{ + "compilerOptions": { + "experimentalDecorators": true, + "useDefineForClassFields": true, + }, + }`, + }), + test(['in.ts', '--outfile=node.js', '--mangle-props=_'].concat(flags), { + 'in.ts': ` + const props: Record = {} + const dec = (n: number) => (_: any, prop: string): void => { + props[n] = prop + } + class Foo { + @dec(1) prop1: any + @dec(2) prop2_: any + @dec(3) ['prop3']: any + @dec(4) ['prop4_']: any + @dec(5) [/* @__KEY__ */ 'prop5']: any + @dec(6) [/* @__KEY__ */ 'prop6_']: any + } + if (props[1] !== 'prop1') throw 'fail 1: ' + props[1] + if (props[2] !== /* @__KEY__ */ 'prop2_') throw 'fail 2: ' + props[2] + if (props[3] !== 'prop3') throw 'fail 3: ' + props[3] + if (props[4] !== 'prop4_') throw 'fail 4: ' + props[4] + if (props[5] !== 'prop5') throw 'fail 5: ' + props[5] + if (props[6] !== /* @__KEY__ */ 'prop6_') throw 'fail 6: ' + props[6] + `, + 'tsconfig.json': `{ + "compilerOptions": { + "experimentalDecorators": true, + "useDefineForClassFields": false, + }, + }`, + }), ) }