From 78a7a5e1d7fc752e6f189b101721474d9810ab42 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 25 Feb 2022 17:06:42 -0800 Subject: [PATCH] Move `module.exports` assigment to the top When using cjs format make sure to move `module.exports` to the top of the generated chunk, right after the `__export()` call: var exports = {}; __export(exports, { // ... }); module.exports = __toCommonJS(exports); This guarantees that all circular requires following these calls would see the correct `module.exports` object. Additionally, add an extra argument for `__reExport` to copy the data to the `module.exports`: __reExport(exports, module.exports, ...) This is required to support `export * from '..'` since now `module.exports` wouldn't automatically receive properties otherwise. Fix: #1894 --- internal/bundler/linker.go | 63 ++++++++++++------- .../bundler/snapshots/snapshots_default.txt | 8 +-- .../snapshots/snapshots_importstar.txt | 30 ++++----- .../bundler/snapshots/snapshots_loader.txt | 10 +-- internal/runtime/runtime.go | 8 ++- scripts/end-to-end-tests.js | 5 +- 6 files changed, 71 insertions(+), 53 deletions(-) diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index d44898f6e88..8f9ff5f15a0 100644 --- a/internal/bundler/linker.go +++ b/internal/bundler/linker.go @@ -2024,6 +2024,32 @@ func (c *linkerContext) createExportsForFile(sourceIndex uint32) { repr.AST.UsesExportsRef = true } + // Decorate "module.exports" with the "__esModule" flag to indicate that + // we used to be an ES module. This is done by wrapping the exports object + // instead of by mutating the exports object because other modules in the + // bundle (including the entry point module) may do "import * as" to get + // access to the exports object and should NOT see the "__esModule" flag. + if repr.Meta.NeedsExportsVariable && + repr.Meta.ForceIncludeExportsForEntryPoint && + c.options.OutputFormat == config.FormatCommonJS { + + runtimeRepr := c.graph.Files[runtime.SourceIndex].InputFile.Repr.(*graph.JSRepr) + toCommonJSRef := runtimeRepr.AST.NamedExports["__toCommonJS"].Ref + + // "module.exports = __toCommonJS(exports);" + nsExportStmts = append(nsExportStmts, js_ast.AssignStmt( + js_ast.Expr{Data: &js_ast.EDot{ + Target: js_ast.Expr{Data: &js_ast.EIdentifier{Ref: c.unboundModuleRef}}, + Name: "exports", + }}, + + js_ast.Expr{Data: &js_ast.ECall{ + Target: js_ast.Expr{Data: &js_ast.EIdentifier{Ref: toCommonJSRef}}, + Args: []js_ast.Expr{{Data: &js_ast.EIdentifier{Ref: repr.AST.ExportsRef}}}, + }}, + )) + } + // No need to generate a part if it'll be empty if len(nsExportStmts) > 0 { // Initialize the part that was allocated for us earlier. The information @@ -3492,6 +3518,16 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL repr := file.InputFile.Repr.(*graph.JSRepr) shouldExtractESMStmtsForWrap := repr.Meta.Wrap != graph.WrapNone + var moduleExportsOrNil js_ast.Expr + if c.options.OutputFormat == config.FormatCommonJS { + moduleExportsOrNil = js_ast.Expr{Data: &js_ast.EDot{ + Target: js_ast.Expr{Data: &js_ast.EIdentifier{Ref: c.unboundModuleRef}}, + Name: "exports", + }} + } else { + moduleExportsOrNil = js_ast.Expr{Data: js_ast.ENullShared} + } + for _, stmt := range partStmts { switch s := stmt.Data.(type) { case *js_ast.SImport: @@ -3547,7 +3583,7 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL ImportRecordIndex: s.ImportRecordIndex, } - // Prefix this module with "__reExport(exports, ns)" + // Prefix this module with "__reExport(exports, module.exports, ns)" exportStarRef := c.graph.Files[runtime.SourceIndex].InputFile.Repr.(*graph.JSRepr).AST.ModuleScope.Members["__reExport"].Ref stmtList.insideWrapperPrefix = append(stmtList.insideWrapperPrefix, js_ast.Stmt{ Loc: stmt.Loc, @@ -3555,6 +3591,7 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL Target: js_ast.Expr{Loc: stmt.Loc, Data: &js_ast.EIdentifier{Ref: exportStarRef}}, Args: []js_ast.Expr{ {Loc: stmt.Loc, Data: &js_ast.EIdentifier{Ref: repr.AST.ExportsRef}}, + moduleExportsOrNil, {Loc: stmt.Loc, Data: &js_ast.EIdentifier{Ref: s.NamespaceRef}}, }, }}}, @@ -3579,12 +3616,12 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL var target js_ast.E if record.SourceIndex.IsValid() { if otherRepr := c.graph.Files[record.SourceIndex.GetIndex()].InputFile.Repr.(*graph.JSRepr); otherRepr.AST.ExportsKind == js_ast.ExportsESMWithDynamicFallback { - // Prefix this module with "__reExport(exports, otherExports)" + // Prefix this module with "__reExport(exports, module.exports, otherExports)" target = &js_ast.EIdentifier{Ref: otherRepr.AST.ExportsRef} } } if target == nil { - // Prefix this module with "__reExport(exports, require(path))" + // Prefix this module with "__reExport(exports, module.exports, require(path))" target = &js_ast.ERequireString{ ImportRecordIndex: s.ImportRecordIndex, } @@ -3596,6 +3633,7 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL Target: js_ast.Expr{Loc: stmt.Loc, Data: &js_ast.EIdentifier{Ref: exportStarRef}}, Args: []js_ast.Expr{ {Loc: stmt.Loc, Data: &js_ast.EIdentifier{Ref: repr.AST.ExportsRef}}, + moduleExportsOrNil, {Loc: record.Range.Loc, Data: target}, }, }}}, @@ -4097,25 +4135,6 @@ func (c *linkerContext) generateEntryPointTailJS( Target: js_ast.Expr{Data: &js_ast.EIdentifier{Ref: repr.AST.WrapperRef}}, }}}}) } - - // Decorate "module.exports" with the "__esModule" flag to indicate that - // we used to be an ES module. This is done by wrapping the exports object - // instead of by mutating the exports object because other modules in the - // bundle (including the entry point module) may do "import * as" to get - // access to the exports object and should NOT see the "__esModule" flag. - if repr.Meta.ForceIncludeExportsForEntryPoint { - // "module.exports = __toCommonJS(exports);" - stmts = append(stmts, js_ast.AssignStmt( - js_ast.Expr{Data: &js_ast.EDot{ - Target: js_ast.Expr{Data: &js_ast.EIdentifier{Ref: c.unboundModuleRef}}, - Name: "exports", - }}, - js_ast.Expr{Data: &js_ast.ECall{ - Target: js_ast.Expr{Data: &js_ast.EIdentifier{Ref: toCommonJSRef}}, - Args: []js_ast.Expr{{Data: &js_ast.EIdentifier{Ref: repr.AST.ExportsRef}}}, - }}, - )) - } } // If we are generating CommonJS for node, encode the known export names in diff --git a/internal/bundler/snapshots/snapshots_default.txt b/internal/bundler/snapshots/snapshots_default.txt index 5ed6ae7bbb9..ddd9d516df5 100644 --- a/internal/bundler/snapshots/snapshots_default.txt +++ b/internal/bundler/snapshots/snapshots_default.txt @@ -857,8 +857,8 @@ TestExportWildcardFSNodeCommonJS ---------- /out.js ---------- // entry.js var entry_exports = {}; -__reExport(entry_exports, require("fs")); module.exports = __toCommonJS(entry_exports); +__reExport(entry_exports, module.exports, require("fs")); ================================================================================ TestExportWildcardFSNodeES6 @@ -938,7 +938,7 @@ var e_exports = {}; import * as x_star from "x"; var init_e = __esm({ "e.js"() { - __reExport(e_exports, x_star); + __reExport(e_exports, null, x_star); } }); @@ -2364,11 +2364,11 @@ __export(entry_exports, { bar: () => import_bar.default, foo: () => import_foo.default }); +module.exports = __toCommonJS(entry_exports); var import_foo = __toESM(require("foo")); // bar.js var import_bar = __toESM(require("bar")); -module.exports = __toCommonJS(entry_exports); ================================================================================ TestReExportDefaultExternalES6 @@ -2410,9 +2410,9 @@ __export(entry_exports, { bar: () => import_bar.default, foo: () => import_foo.default }); +module.exports = __toCommonJS(entry_exports); var import_foo = __toESM(require("./foo")); var import_bar = __toESM(require("./bar")); -module.exports = __toCommonJS(entry_exports); ================================================================================ TestReExportDefaultNoBundleES6 diff --git a/internal/bundler/snapshots/snapshots_importstar.txt b/internal/bundler/snapshots/snapshots_importstar.txt index 23be9e99c8a..5cc3740566e 100644 --- a/internal/bundler/snapshots/snapshots_importstar.txt +++ b/internal/bundler/snapshots/snapshots_importstar.txt @@ -12,8 +12,8 @@ var entry_exports = {}; __export(entry_exports, { ns: () => ns }); -var ns = __toESM(require_foo()); module.exports = __toCommonJS(entry_exports); +var ns = __toESM(require_foo()); ================================================================================ TestExportOtherCommonJS @@ -30,8 +30,8 @@ var entry_exports = {}; __export(entry_exports, { bar: () => import_foo.bar }); -var import_foo = __toESM(require_foo()); module.exports = __toCommonJS(entry_exports); +var import_foo = __toESM(require_foo()); ================================================================================ TestExportOtherNestedCommonJS @@ -48,10 +48,10 @@ var entry_exports = {}; __export(entry_exports, { y: () => import_foo.x }); +module.exports = __toCommonJS(entry_exports); // bar.js var import_foo = __toESM(require_foo()); -module.exports = __toCommonJS(entry_exports); ================================================================================ TestExportSelfAndImportSelfCommonJS @@ -61,9 +61,9 @@ var entry_exports = {}; __export(entry_exports, { foo: () => foo }); +module.exports = __toCommonJS(entry_exports); var foo = 123; console.log(entry_exports); -module.exports = __toCommonJS(entry_exports); ================================================================================ TestExportSelfAndRequireSelfCommonJS @@ -73,6 +73,7 @@ var entry_exports = {}; __export(entry_exports, { foo: () => foo }); +module.exports = __toCommonJS(entry_exports); var foo; var init_entry = __esm({ "entry.js"() { @@ -81,7 +82,6 @@ var init_entry = __esm({ } }); init_entry(); -module.exports = __toCommonJS(entry_exports); ================================================================================ TestExportSelfAsNamespaceCommonJS @@ -92,8 +92,8 @@ __export(entry_exports, { foo: () => foo, ns: () => entry_exports }); -var foo = 123; module.exports = __toCommonJS(entry_exports); +var foo = 123; ================================================================================ TestExportSelfAsNamespaceES6 @@ -118,8 +118,8 @@ var entry_exports = {}; __export(entry_exports, { foo: () => foo }); -var foo = 123; module.exports = __toCommonJS(entry_exports); +var foo = 123; ================================================================================ TestExportSelfCommonJSMinified @@ -169,10 +169,10 @@ var entry_exports = {}; __export(entry_exports, { foo: () => foo }); +module.exports = __toCommonJS(entry_exports); // foo.js var foo = "foo"; -module.exports = __toCommonJS(entry_exports); ================================================================================ TestImportDefaultNamespaceComboIssue446 @@ -275,8 +275,8 @@ var entry_exports = {}; __export(entry_exports, { ns: () => ns }); -var ns = __toESM(require_foo()); module.exports = __toCommonJS(entry_exports); +var ns = __toESM(require_foo()); ================================================================================ TestImportExportSelfAsNamespaceES6 @@ -869,8 +869,8 @@ var entry_exports = {}; __export(entry_exports, { out: () => out }); -var out = __toESM(require("foo")); module.exports = __toCommonJS(entry_exports); +var out = __toESM(require("foo")); ================================================================================ TestReExportStarAsES6NoBundle @@ -888,8 +888,8 @@ var entry_exports = {}; __export(entry_exports, { out: () => out }); -var out = __toESM(require("foo")); module.exports = __toCommonJS(entry_exports); +var out = __toESM(require("foo")); ================================================================================ TestReExportStarAsExternalES6 @@ -929,8 +929,8 @@ var mod = (() => { TestReExportStarCommonJSNoBundle ---------- /out.js ---------- var entry_exports = {}; -__reExport(entry_exports, require("foo")); module.exports = __toCommonJS(entry_exports); +__reExport(entry_exports, module.exports, require("foo")); ================================================================================ TestReExportStarES6NoBundle @@ -942,8 +942,8 @@ TestReExportStarExternalCommonJS ---------- /out.js ---------- // entry.js var entry_exports = {}; -__reExport(entry_exports, require("foo")); module.exports = __toCommonJS(entry_exports); +__reExport(entry_exports, module.exports, require("foo")); ================================================================================ TestReExportStarExternalES6 @@ -957,7 +957,7 @@ TestReExportStarExternalIIFE var mod = (() => { // entry.js var entry_exports = {}; - __reExport(entry_exports, __require("foo")); + __reExport(entry_exports, null, __require("foo")); return __toCommonJS(entry_exports); })(); @@ -966,7 +966,7 @@ TestReExportStarIIFENoBundle ---------- /out.js ---------- var mod = (() => { var entry_exports = {}; - __reExport(entry_exports, require("foo")); + __reExport(entry_exports, null, require("foo")); return __toCommonJS(entry_exports); })(); diff --git a/internal/bundler/snapshots/snapshots_loader.txt b/internal/bundler/snapshots/snapshots_loader.txt index 155a5df0f95..c8e0a128d88 100644 --- a/internal/bundler/snapshots/snapshots_loader.txt +++ b/internal/bundler/snapshots/snapshots_loader.txt @@ -903,9 +903,10 @@ export default { TestToESMWrapperOmission ---------- /out/entry.js ---------- var entry_exports = {}; +module.exports = __toCommonJS(entry_exports); var import_a_nowrap = require("a_nowrap"); var import_b_nowrap = require("b_nowrap"); -__reExport(entry_exports, require("c_nowrap")); +__reExport(entry_exports, module.exports, require("c_nowrap")); var d = __toESM(require("d_WRAP")); var import_e_WRAP = __toESM(require("e_WRAP")); var import_f_WRAP = __toESM(require("f_WRAP")); @@ -922,7 +923,6 @@ x = h; i.x(); j.x``; x = Promise.resolve().then(() => __toESM(require("k_WRAP"))); -module.exports = __toCommonJS(entry_exports); ================================================================================ TestWarnCommonJSExportsInESMBundle @@ -932,10 +932,10 @@ var cjs_in_esm_exports = {}; __export(cjs_in_esm_exports, { foo: () => foo }); +module.exports = __toCommonJS(cjs_in_esm_exports); var foo = 1; exports.foo = 2; module.exports = 3; -module.exports = __toCommonJS(cjs_in_esm_exports); ---------- /out/import-in-cjs.js ---------- // import-in-cjs.js @@ -954,19 +954,19 @@ var cjs_in_esm_exports = {}; __export(cjs_in_esm_exports, { foo: () => foo }); +module.exports = __toCommonJS(cjs_in_esm_exports); let foo = 1; exports.foo = 2; module.exports = 3; -module.exports = __toCommonJS(cjs_in_esm_exports); ---------- /out/cjs-in-esm2.js ---------- var cjs_in_esm2_exports = {}; __export(cjs_in_esm2_exports, { foo: () => foo }); +module.exports = __toCommonJS(cjs_in_esm2_exports); let foo = 1; module.exports.bar = 3; -module.exports = __toCommonJS(cjs_in_esm2_exports); ---------- /out/import-in-cjs.js ---------- var import_bar = require("bar"); diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index bd6eb01c9d2..70089b5493b 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -185,7 +185,9 @@ func code(isES6 bool) string { for (var name in all) __defProp(target, name, { get: all[name], enumerable: true }) } - export var __reExport = (target, module, copyDefault, desc) => { + export var __reExport = (target, secondTarget, module, copyDefault, desc) => { + if (secondTarget && secondTarget !== target) + __reExport(secondTarget, null, module, copyDefault, desc) if (module && typeof module === 'object' || typeof module === 'function') ` @@ -225,14 +227,14 @@ func code(isES6 bool) string { !isNodeMode && module && module.__esModule ? { get: () => module.default, enumerable: true } : { value: module, enumerable: true }) - ), module) + ), null, module) } // Converts the module from ESM to CommonJS export var __toCommonJS = /* @__PURE__ */ (cache => { return (module, temp) => { return (cache && cache.get(module)) || ( - temp = __reExport(__markAsModule({}), module, /* copyDefault */ 1), + temp = __reExport(__markAsModule({}), null, module, /* copyDefault */ 1), cache && cache.set(module, temp), temp) } diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 7f5931eafaa..985f4491348 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -1101,10 +1101,7 @@ import {a} from './re-export' let fn = a() - // Note: The "void 0" is different here. This case broke when fixing - // something else ("default" export semantics in node). This test still - // exists to document this broken behavior. - if (fn === a || fn() !== void 0) throw 'fail' + if (fn === a) throw 'fail' `, 're-export.ts': ` export * from './a'