From 00237e723893a99ee1819ee32d8f19dd8e315fbd Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 3 Jun 2021 23:26:21 -0700 Subject: [PATCH] fix for "export default class" transform (#1346) --- CHANGELOG.md | 4 ++++ .../bundler/snapshots/snapshots_default.txt | 10 ++++------ .../bundler/snapshots/snapshots_lower.txt | 8 ++------ internal/bundler/snapshots/snapshots_ts.txt | 9 +-------- internal/js_parser/js_parser_lower.go | 20 ++++++------------- scripts/end-to-end-tests.js | 8 ++++++++ 6 files changed, 25 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea1d59b270c..c6ec72a46f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,10 @@ Importing CSS from JS when bundling causes esbuild to generate a sibling CSS output file next to the resulting JS output file containing the bundled CSS. The order of the imported CSS files in the output was accidentally the inverse order of the order in which the JS files were evaluated. Instead the order of the imported CSS files should match the order in which the JS files were evaluated. This fix was contributed by [@dmitrage](https://github.com/dmitrage). +* Fix an edge case with transforming `export default class` ([#1346](https://github.com/evanw/esbuild/issues/1346)) + + Statements of the form `export default class x {}` were incorrectly transformed to `class x {} var y = x; export {y as default}` instead of `class x {} export {x as default}`. Transforming these statements like this is incorrect in the rare case that the class is later reassigned by name within the same file such as `export default class x {} x = null`. Here the imported value should be `null` but was incorrectly the class object instead. This is unlikely to matter in real-world code but it has still been fixed to improve correctness. + ## 0.12.5 * Add support for lowering tagged template literals to ES5 ([#297](https://github.com/evanw/esbuild/issues/297)) diff --git a/internal/bundler/snapshots/snapshots_default.txt b/internal/bundler/snapshots/snapshots_default.txt index 6fa43a56ee6..0a51ca476bd 100644 --- a/internal/bundler/snapshots/snapshots_default.txt +++ b/internal/bundler/snapshots/snapshots_default.txt @@ -590,28 +590,26 @@ var init_commonjs = __esm({ // c.js var c_exports = {}; __export(c_exports, { - default: () => c_default2 + default: () => c_default }); -var c_default, c_default2; +var c_default; var init_c = __esm({ "c.js"() { c_default = class { }; - c_default2 = c_default; } }); // d.js var d_exports = {}; __export(d_exports, { - default: () => d_default + default: () => Foo }); -var Foo, d_default; +var Foo; var init_d = __esm({ "d.js"() { Foo = class { }; - d_default = Foo; Foo.prop = 123; } }); diff --git a/internal/bundler/snapshots/snapshots_lower.txt b/internal/bundler/snapshots/snapshots_lower.txt index d4789fdebe2..0860b8eafc4 100644 --- a/internal/bundler/snapshots/snapshots_lower.txt +++ b/internal/bundler/snapshots/snapshots_lower.txt @@ -188,7 +188,6 @@ var loose_default = class { __publicField(this, "foo"); } }; -var loose_default2 = loose_default; // strict/index.js var strict_default = class { @@ -196,10 +195,9 @@ var strict_default = class { __publicField(this, "foo"); } }; -var strict_default2 = strict_default; // entry.js -console.log(loose_default2, strict_default2); +console.log(loose_default, strict_default); ================================================================================ TestLowerExportStarAsNameCollision @@ -1048,7 +1046,6 @@ TestTSLowerClassFieldStrictTsconfigJson2020 // loose/index.ts var loose_default = class { }; -var loose_default2 = loose_default; // strict/index.ts var strict_default = class { @@ -1056,10 +1053,9 @@ var strict_default = class { __publicField(this, "foo"); } }; -var strict_default2 = strict_default; // entry.js -console.log(loose_default2, strict_default2); +console.log(loose_default, strict_default); ================================================================================ TestTSLowerClassPrivateFieldNextNoBundle diff --git a/internal/bundler/snapshots/snapshots_ts.txt b/internal/bundler/snapshots/snapshots_ts.txt index 5c4ebf82bf5..2871fba413e 100644 --- a/internal/bundler/snapshots/snapshots_ts.txt +++ b/internal/bundler/snapshots/snapshots_ts.txt @@ -406,7 +406,6 @@ Foo = __decorateClass([ __decorateParam(1, x1), __decorateParam(1, y1) ], Foo); -var all_default = Foo; // all_computed.ts var _a, _b, _c, _d, _e, _f, _g, _h; @@ -460,7 +459,6 @@ Foo2 = __decorateClass([ x?.[_ + "y"](), new y?.[_ + "x"]() ], Foo2); -var all_computed_default = Foo2; // a.ts var a_class = class { @@ -519,7 +517,6 @@ e_default = __decorateClass([ x(() => 0), y(() => 1) ], e_default); -var e_default2 = e_default; // f.ts var f = class { @@ -532,7 +529,6 @@ f = __decorateClass([ x(() => 0), y(() => 1) ], f); -var f_default = f; // g.ts var g_default = class { @@ -541,7 +537,6 @@ g_default = __decorateClass([ x(() => 0), y(() => 1) ], g_default); -var g_default2 = g_default; // h.ts var h = class { @@ -554,7 +549,6 @@ h = __decorateClass([ x(() => 0), y(() => 1) ], h); -var h_default = h; // i.ts var i_class = class { @@ -584,7 +578,6 @@ __decorateClass([ __decorateParam(0, x2(() => 0)), __decorateParam(0, y(() => 1)) ], k_default.prototype, "foo", 1); -var k_default2 = k_default; // arguments.ts function dec(x2) { @@ -602,7 +595,7 @@ function fn(x2) { } // entry.js -console.log(all_default, all_computed_default, a, b, c, d, e_default2, f_default, g_default2, h_default, i, j, k_default2, fn); +console.log(Foo, Foo2, a, b, c, d, e_default, f, g_default, h, i, j, k_default, fn); ================================================================================ TestTypeScriptDecoratorsKeepNames diff --git a/internal/js_parser/js_parser_lower.go b/internal/js_parser/js_parser_lower.go index 5514dc21db6..e08aa86e60d 100644 --- a/internal/js_parser/js_parser_lower.go +++ b/internal/js_parser/js_parser_lower.go @@ -1994,7 +1994,7 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, shadowRef js_ast if kind == classKindExportDefaultStmt { class.Name = &defaultName } else { - class.Name = &js_ast.LocRef{Loc: classLoc, Ref: p.generateTempRef(tempRefNoDeclare, "zomzomz")} + class.Name = &js_ast.LocRef{Loc: classLoc, Ref: p.generateTempRef(tempRefNoDeclare, "")} } } p.recordUsage(class.Name.Ref) @@ -2568,25 +2568,17 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, shadowRef js_ast p.recordUsage(nameForClassDecorators.Ref) } if generatedLocalStmt { + // "export default class x {}" => "class x {} export {x as default}" if kind == classKindExportDefaultStmt { - // Generate a new default name symbol since the current one is being used - // by the class. If this SExportDefault turns into a variable declaration, - // we don't want it to accidentally use the same variable as the class and - // cause a name collision. - defaultRef := p.generateTempRef(tempRefNoDeclare, p.source.IdentifierName+"_default") - p.recordDeclaredSymbol(defaultRef) - - name := nameFunc() - stmts = append(stmts, js_ast.Stmt{Loc: classLoc, Data: &js_ast.SExportDefault{ - DefaultName: js_ast.LocRef{Loc: defaultName.Loc, Ref: defaultRef}, - Value: js_ast.Stmt{Loc: name.Loc, Data: &js_ast.SExpr{Value: name}}, + stmts = append(stmts, js_ast.Stmt{Loc: classLoc, Data: &js_ast.SExportClause{ + Items: []js_ast.ClauseItem{{Alias: "default", Name: defaultName}}, }}) } // Calling "nameFunc" will set the class name, but we don't want it to have // one. If the class name was necessary, we would have already split it off - // into a "const" symbol above. Reset it back to empty here now that we - // know we won't call "nameFunc" after this point. + // into a variable above. Reset it back to empty here now that we know we + // won't call "nameFunc" after this point. class.Name = nil } return stmts, js_ast.Expr{} diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 7739e627e76..a787b823539 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -761,6 +761,14 @@ 'in.js': `const out = require('./foo'); if (out.__esModule || out.default !== 123) throw 'fail'`, 'foo.js': `export default 123`, }), + test(['--bundle', 'in.js', '--outfile=node.js'], { + 'in.js': `const out = require('./foo'); if (out.__esModule || out.default !== null) throw 'fail'`, + 'foo.js': `export default function x() {} x = null`, + }), + test(['--bundle', 'in.js', '--outfile=node.js'], { + 'in.js': `const out = require('./foo'); if (out.__esModule || out.default !== null) throw 'fail'`, + 'foo.js': `export default class x {} x = null`, + }), // Self export test(['--bundle', 'in.js', '--outfile=node.js'], {