From 6e4b478940ac1ac929567878cbc0d93fe15df0b4 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Fri, 21 Feb 2020 20:57:48 +0100 Subject: [PATCH] improve exports hoisting in concatenated modules #10409 --- lib/optimize/ConcatenatedModule.js | 362 +++++++----------- .../__snapshots__/StatsTestCases.test.js.snap | 4 +- .../cases/errors/harmony-import-missing2/a.js | 1 + .../errors/harmony-import-missing2/errors.js | 4 + .../errors/harmony-import-missing2/index.js | 11 + .../errors/harmony-import-missing2/module1.js | 2 + .../errors/harmony-import-missing2/module2.js | 2 + 7 files changed, 154 insertions(+), 232 deletions(-) create mode 100644 test/cases/errors/harmony-import-missing2/a.js create mode 100644 test/cases/errors/harmony-import-missing2/errors.js create mode 100644 test/cases/errors/harmony-import-missing2/index.js create mode 100644 test/cases/errors/harmony-import-missing2/module1.js create mode 100644 test/cases/errors/harmony-import-missing2/module2.js diff --git a/lib/optimize/ConcatenatedModule.js b/lib/optimize/ConcatenatedModule.js index ef3dc8c8a..a1d0bf56f 100644 --- a/lib/optimize/ConcatenatedModule.js +++ b/lib/optimize/ConcatenatedModule.js @@ -602,6 +602,48 @@ const matchModuleReference = (name, modulesWithInfo) => { const TYPES = new Set(["javascript"]); +const getHarmonyExportImportedSpecifierDependencyExports = ( + dep, + moduleGraph +) => { + const importModule = moduleGraph.getModule(dep); + if (!importModule) return []; + const ids = dep.getIds(moduleGraph); + if (ids.length > 0) { + // export { named } from "module" + return [ + { + name: dep.name, + ids + } + ]; + } + if (dep.name) { + // export * as abc from "module" + return [ + { + name: dep.name, + ids: [] + } + ]; + } + // export * from "module" + const providedExports = moduleGraph.getProvidedExports(importModule); + if (Array.isArray(providedExports)) { + return providedExports + .filter(exp => exp !== "default" && !dep.activeExports.has(exp)) + .map(exp => { + return { + name: exp, + ids: [exp] + }; + }); + } + + // unknown, should not happen + throw new Error("ConcatenatedModule: unknown exports"); +}; + class ConcatenatedModule extends Module { /** * @param {Module} rootModule the root module of the concatenation @@ -956,20 +998,10 @@ class ConcatenatedModule extends Module { // Create mapping from module to info const moduleToInfoMap = modulesWithInfoToMap(modulesWithInfo); - // Map with all root exposed used exports - /** @type {Map} */ - const exportsMap = new Map(); - - // Set with all root exposed unused exports - /** @type {Set} */ - const unusedExports = new Set(); - // Configure template decorators for dependencies const innerDependencyTemplates = this._getInnerDependencyTemplates( dependencyTemplates, - moduleToInfoMap, - exportsMap, - unusedExports + moduleToInfoMap ); // Generate source code and analyse scopes @@ -1215,6 +1247,87 @@ class ConcatenatedModule extends Module { } } + // Map with all root exposed used exports + /** @type {Map} */ + const exportsMap = new Map(); + + // Set with all root exposed unused exports + /** @type {Set} */ + const unusedExports = new Set(); + + for (const dep of this.rootModule.dependencies) { + if (dep instanceof HarmonyExportSpecifierDependency) { + const used = /** @type {string | false } */ (this.rootModule.getUsedName( + moduleGraph, + dep.name + )); + if (used) { + const info = moduleToInfoMap.get(this.rootModule); + if (!exportsMap.has(used)) { + exportsMap.set( + used, + () => `/* binding */ ${info.internalNames.get(dep.id)}` + ); + } + } else { + unusedExports.add(dep.name || "namespace"); + } + } else if (dep instanceof HarmonyExportExpressionDependency) { + const used = /** @type {string | false } */ (this.rootModule.getUsedName( + moduleGraph, + "default" + )); + if (used) { + const info = moduleToInfoMap.get(this.rootModule); + if (!exportsMap.has(used)) { + exportsMap.set( + used, + () => + `/* default */ ${info.internalNames.get( + "__WEBPACK_MODULE_DEFAULT_EXPORT__" + )}` + ); + } + } else { + unusedExports.add("default"); + } + } else if (dep instanceof HarmonyExportImportedSpecifierDependency) { + const exportDefs = getHarmonyExportImportedSpecifierDependencyExports( + dep, + moduleGraph + ); + for (const def of exportDefs) { + const importedModule = moduleGraph.getModule(dep); + const info = moduleToInfoMap.get(importedModule); + const used = /** @type {string | false } */ (this.rootModule.getUsedName( + moduleGraph, + def.name + )); + if (used) { + if (!exportsMap.has(used)) { + exportsMap.set(used, requestShortener => { + const finalName = getFinalName( + moduleGraph, + info, + def.ids, + moduleToInfoMap, + requestShortener, + runtimeTemplate, + false, + false, + this.rootModule.buildMeta.strictHarmonyModule, + true + ); + return `/* reexport */ ${finalName}`; + }); + } + } else { + unusedExports.add(def.name); + } + } + } + } + const result = new ConcatSource(); // add harmony compatibility flag (must be first because of possible circular dependencies) @@ -1222,7 +1335,7 @@ class ConcatenatedModule extends Module { moduleGraph.getExportsInfo(this).otherExportsInfo.used !== UsageState.Unused ) { - result.add(`/* ESM COMPAT FLAG */\n`); + result.add(`// ESM COMPAT FLAG\n`); result.add( runtimeTemplate.defineEsModuleFlagStatement({ exportsArgument: this.exportsArgument, @@ -1508,16 +1621,9 @@ class ConcatenatedModule extends Module { /** * @param {DependencyTemplates} dependencyTemplates outer dependency templates * @param {Map} moduleToInfoMap map for module info - * @param {Map} exportsMap mapping from used name to exposed variable name - * @param {Set} unusedExports list of unused export names * @returns {DependencyTemplates} inner dependency templates */ - _getInnerDependencyTemplates( - dependencyTemplates, - moduleToInfoMap, - exportsMap, - unusedExports - ) { + _getInnerDependencyTemplates(dependencyTemplates, moduleToInfoMap) { const innerDependencyTemplates = dependencyTemplates.clone(); innerDependencyTemplates.set( HarmonyImportSpecifierDependency, @@ -1535,41 +1641,23 @@ class ConcatenatedModule extends Module { ); innerDependencyTemplates.set( HarmonyExportSpecifierDependency, - new HarmonyExportSpecifierDependencyConcatenatedTemplate( - dependencyTemplates.get(HarmonyExportSpecifierDependency), - this.rootModule, - moduleToInfoMap, - exportsMap, - unusedExports - ) + new NullTemplate() ); innerDependencyTemplates.set( HarmonyExportExpressionDependency, new HarmonyExportExpressionDependencyConcatenatedTemplate( dependencyTemplates.get(HarmonyExportExpressionDependency), this.rootModule, - moduleToInfoMap, - exportsMap, - unusedExports + moduleToInfoMap ) ); innerDependencyTemplates.set( HarmonyExportImportedSpecifierDependency, - new HarmonyExportImportedSpecifierDependencyConcatenatedTemplate( - dependencyTemplates.get(HarmonyExportImportedSpecifierDependency), - this.rootModule, - moduleToInfoMap, - exportsMap, - unusedExports - ) + new NullTemplate() ); innerDependencyTemplates.set( HarmonyCompatibilityDependency, - new HarmonyCompatibilityDependencyConcatenatedTemplate( - dependencyTemplates.get(HarmonyCompatibilityDependency), - this.rootModule, - moduleToInfoMap - ) + new NullTemplate() ); // Must use full identifier in our cache here to ensure that the source // is updated should our dependencies list change. @@ -1739,47 +1827,6 @@ class HarmonyImportSideEffectDependencyConcatenatedTemplate extends DependencyTe } } -class HarmonyExportSpecifierDependencyConcatenatedTemplate extends DependencyTemplate { - constructor( - originalTemplate, - rootModule, - modulesMap, - exportsMap, - unusedExports - ) { - super(); - this.originalTemplate = originalTemplate; - this.rootModule = rootModule; - this.modulesMap = modulesMap; - this.exportsMap = exportsMap; - this.unusedExports = unusedExports; - } - - /** - * @param {Dependency} dependency the dependency for which the template should be applied - * @param {ReplaceSource} source the current replace source which can be modified - * @param {DependencyTemplateContext} templateContext the context object - * @returns {void} - */ - apply(dependency, source, { module, moduleGraph }) { - const dep = /** @type {HarmonyExportSpecifierDependency} */ (dependency); - if (module === this.rootModule) { - const used = module.getUsedName(moduleGraph, dep.name); - if (used) { - const info = this.modulesMap.get(module); - if (!this.exportsMap.has(used)) { - this.exportsMap.set( - used, - () => `/* binding */ ${info.internalNames.get(dep.id)}` - ); - } - } else { - this.unusedExports.add(dep.name || "namespace"); - } - } - } -} - class HarmonyExportExpressionDependencyConcatenatedTemplate extends DependencyTemplate { constructor( originalTemplate, @@ -1809,24 +1856,6 @@ class HarmonyExportExpressionDependencyConcatenatedTemplate extends DependencyTe ) { const dep = /** @type {HarmonyExportExpressionDependency} */ (dependency); - if (module === this.rootModule) { - const used = module.getUsedName(moduleGraph, "default"); - if (used) { - const info = this.modulesMap.get(module); - if (!this.exportsMap.has(used)) { - this.exportsMap.set( - used, - () => - `/* default */ ${info.internalNames.get( - "__WEBPACK_MODULE_DEFAULT_EXPORT__" - )}` - ); - } - } else { - this.unusedExports.add("default"); - } - } - const content = `/* harmony default export */ ${ runtimeTemplate.supportsConst() ? "const" : "var" } __WEBPACK_MODULE_DEFAULT_EXPORT__ = `; @@ -1849,135 +1878,8 @@ class HarmonyExportExpressionDependencyConcatenatedTemplate extends DependencyTe } } -class HarmonyExportImportedSpecifierDependencyConcatenatedTemplate extends DependencyTemplate { - constructor( - originalTemplate, - rootModule, - modulesMap, - exportsMap, - unusedExports - ) { - super(); - this.originalTemplate = originalTemplate; - this.rootModule = rootModule; - this.modulesMap = modulesMap; - this.exportsMap = exportsMap; - this.unusedExports = unusedExports; - } - - /** - * @typedef {Object} GetExportsResultItem - * @property {string} name - * @property {string[]} ids - */ - - /** - * @param {HarmonyExportImportedSpecifierDependency} dep dependency - * @param {DependencyTemplateContext} templateContext template context - * @returns {GetExportsResultItem[]} exports - */ - getExports(dep, { moduleGraph }) { - const importModule = moduleGraph.getModule(dep); - const ids = dep.getIds(moduleGraph); - if (ids.length > 0) { - // export { named } from "module" - return [ - { - name: dep.name, - ids - } - ]; - } - if (dep.name) { - // export * as abc from "module" - return [ - { - name: dep.name, - ids: [] - } - ]; - } - // export * from "module" - const providedExports = moduleGraph.getProvidedExports(importModule); - if (Array.isArray(providedExports)) { - return providedExports - .filter(exp => exp !== "default" && !dep.activeExports.has(exp)) - .map(exp => { - return { - name: exp, - ids: [exp] - }; - }); - } - - // unknown, should not happen - throw new Error("ConcatenatedModule: unknown exports"); - } - - /** - * @param {Dependency} dependency the dependency for which the template should be applied - * @param {ReplaceSource} source the current replace source which can be modified - * @param {DependencyTemplateContext} templateContext the context object - * @returns {void} - */ - apply(dependency, source, templateContext) { - const { module, moduleGraph, runtimeTemplate } = templateContext; - const dep = /** @type {HarmonyExportImportedSpecifierDependency} */ (dependency); - const importedModule = moduleGraph.getModule(dep); - const info = this.modulesMap.get(importedModule); - if (!info) { - this.originalTemplate.apply(dependency, source, templateContext); - } else if (module === this.rootModule) { - const exportDefs = this.getExports(dep, templateContext); - for (const def of exportDefs) { - const used = module.getUsedName(moduleGraph, def.name); - if (used) { - if (!this.exportsMap.has(used)) { - this.exportsMap.set(used, requestShortener => { - const finalName = getFinalName( - moduleGraph, - info, - def.ids, - this.modulesMap, - requestShortener, - runtimeTemplate, - false, - false, - module.buildMeta.strictHarmonyModule, - true - ); - return `/* reexport */ ${finalName}`; - }); - } - } else { - this.unusedExports.add(def.name); - } - } - } - } -} - -class HarmonyCompatibilityDependencyConcatenatedTemplate extends DependencyTemplate { - constructor(originalTemplate, rootModule, modulesMap) { - super(); - this.originalTemplate = originalTemplate; - this.rootModule = rootModule; - this.modulesMap = modulesMap; - } - - /** - * @param {Dependency} dependency the dependency for which the template should be applied - * @param {ReplaceSource} source the current replace source which can be modified - * @param {DependencyTemplateContext} templateContext the context object - * @returns {void} - */ - apply( - dependency, - source, - { runtimeTemplate, dependencyTemplates, moduleGraph } - ) { - // do nothing - } +class NullTemplate { + apply() {} } module.exports = ConcatenatedModule; diff --git a/test/__snapshots__/StatsTestCases.test.js.snap b/test/__snapshots__/StatsTestCases.test.js.snap index 8a15c82f2..2f6cb3d60 100644 --- a/test/__snapshots__/StatsTestCases.test.js.snap +++ b/test/__snapshots__/StatsTestCases.test.js.snap @@ -690,7 +690,7 @@ Child Time: Xms Built at: 1970-04-20 12:42:42 Asset Size - 703-35b05b4f9ece3e5b7253.js 460 bytes [emitted] [immutable] + 703-35b05b4f9ece3e5b7253.js 457 bytes [emitted] [immutable] 703-35b05b4f9ece3e5b7253.js.map 344 bytes [emitted] [dev] main-322147d992bb0a885b10.js 8.02 KiB [emitted] [immutable] [name: main] main-322147d992bb0a885b10.js.map 7.14 KiB [emitted] [dev] [name: (main)] @@ -703,7 +703,7 @@ Child Time: Xms Built at: 1970-04-20 12:42:42 Asset Size - 703-35b05b4f9ece3e5b7253.js 460 bytes [emitted] [immutable] + 703-35b05b4f9ece3e5b7253.js 457 bytes [emitted] [immutable] 703-35b05b4f9ece3e5b7253.js.map 344 bytes [emitted] [dev] main-322147d992bb0a885b10.js 8.02 KiB [emitted] [immutable] [name: main] main-322147d992bb0a885b10.js.map 7.14 KiB [emitted] [dev] [name: (main)] diff --git a/test/cases/errors/harmony-import-missing2/a.js b/test/cases/errors/harmony-import-missing2/a.js new file mode 100644 index 000000000..173df5cb0 --- /dev/null +++ b/test/cases/errors/harmony-import-missing2/a.js @@ -0,0 +1 @@ +export var test = "test"; diff --git a/test/cases/errors/harmony-import-missing2/errors.js b/test/cases/errors/harmony-import-missing2/errors.js new file mode 100644 index 000000000..67d1f2321 --- /dev/null +++ b/test/cases/errors/harmony-import-missing2/errors.js @@ -0,0 +1,4 @@ +module.exports = [ + [/Can't resolve '.\/missing1'/], + [/Can't resolve '.\/missing2'/] +]; diff --git a/test/cases/errors/harmony-import-missing2/index.js b/test/cases/errors/harmony-import-missing2/index.js new file mode 100644 index 000000000..77cb8d2ed --- /dev/null +++ b/test/cases/errors/harmony-import-missing2/index.js @@ -0,0 +1,11 @@ +it("should not crash on importing missing modules", function() { + expect(function() { + require("./module1"); + }).toThrowError(); +}); + +it("should not crash on importing missing modules", function() { + expect(function() { + require("./module2"); + }).toThrowError(); +}); diff --git a/test/cases/errors/harmony-import-missing2/module1.js b/test/cases/errors/harmony-import-missing2/module1.js new file mode 100644 index 000000000..cc6275d16 --- /dev/null +++ b/test/cases/errors/harmony-import-missing2/module1.js @@ -0,0 +1,2 @@ +export * from "./missing1"; +export * from "./a?1"; diff --git a/test/cases/errors/harmony-import-missing2/module2.js b/test/cases/errors/harmony-import-missing2/module2.js new file mode 100644 index 000000000..3b41b6483 --- /dev/null +++ b/test/cases/errors/harmony-import-missing2/module2.js @@ -0,0 +1,2 @@ +export { a } from "./missing2"; +export * from "./a?2";