From c94aea82ccb4490a10c6e638c7b19a6f15327f9c Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Thu, 5 Mar 2020 13:31:58 +0100 Subject: [PATCH] enable export mangling for system.js externals add __esModule flag only when needed --- lib/Dependency.js | 1 + lib/ExternalModule.js | 17 ++- lib/FlagDependencyExportsPlugin.js | 23 +++- lib/ModuleGraph.js | 19 ++- ...armonyExportImportedSpecifierDependency.js | 1 + lib/dependencies/StaticExportsDependency.js | 19 +-- lib/library/SystemLibraryPlugin.js | 109 +++++++++++++----- lib/wasm/WebAssemblyParser.js | 2 +- .../externals/externals-system/index.js | 3 +- .../externals/externals-system/test.config.js | 8 +- .../externals-system/webpack.config.js | 3 +- test/helpers/fakeSystem.js | 3 +- 12 files changed, 147 insertions(+), 61 deletions(-) diff --git a/lib/Dependency.js b/lib/Dependency.js index f7dc9b241..689a8206e 100644 --- a/lib/Dependency.js +++ b/lib/Dependency.js @@ -44,6 +44,7 @@ /** * @typedef {Object} ExportsSpec * @property {(string | ExportSpec)[] | true | null} exports exported names, true for unknown exports or null for no exports + * @property {boolean=} canMangle can the export be renamed (defaults to true) * @property {Module[]=} dependencies module on which the result depends on */ diff --git a/lib/ExternalModule.js b/lib/ExternalModule.js index 434d12fa5..769886a71 100644 --- a/lib/ExternalModule.js +++ b/lib/ExternalModule.js @@ -9,6 +9,7 @@ const { OriginalSource, RawSource } = require("webpack-sources"); const Module = require("./Module"); const RuntimeGlobals = require("./RuntimeGlobals"); const Template = require("./Template"); +const StaticExportsDependency = require("./dependencies/StaticExportsDependency"); const makeSerializable = require("./util/makeSerializable"); const propertyAccess = require("./util/propertyAccess"); @@ -194,10 +195,17 @@ class ExternalModule extends Module { * @returns {void} */ build(options, compilation, resolver, fs, callback) { - this.buildMeta = {}; + this.buildMeta = { + exportsType: undefined + }; this.buildInfo = { strict: true }; + this.clearDependenciesAndBlocks(); + if (this.externalType === "system") { + this.buildMeta.exportsType = "namespace"; + this.addDependency(new StaticExportsDependency(true, true)); + } callback(); } @@ -290,6 +298,13 @@ class ExternalModule extends Module { hash.update( JSON.stringify(Boolean(this.isOptional(chunkGraph.moduleGraph))) ); + if (this.externalType === "system") { + const exportsInfo = chunkGraph.moduleGraph.getExportsInfo(this); + for (const exportInfo of exportsInfo.orderedExports) { + hash.update(exportInfo.name); + hash.update(exportInfo.getUsedName() || ""); + } + } super.updateHash(hash, chunkGraph); } diff --git a/lib/FlagDependencyExportsPlugin.js b/lib/FlagDependencyExportsPlugin.js index bb2386a60..d5ffc168a 100644 --- a/lib/FlagDependencyExportsPlugin.js +++ b/lib/FlagDependencyExportsPlugin.js @@ -113,10 +113,11 @@ class FlagDependencyExportsPlugin { const exportDesc = dep.getExports(moduleGraph); if (!exportDesc) return; const exports = exportDesc.exports; + const canMangle = exportDesc.canMangle; const exportDeps = exportDesc.dependencies; if (exports === true) { // unknown exports - if (exportsInfo.setUnknownExportsProvided()) { + if (exportsInfo.setUnknownExportsProvided(canMangle)) { changed = true; } } else if (Array.isArray(exports)) { @@ -131,6 +132,13 @@ class FlagDependencyExportsPlugin { exportInfo.provided = true; changed = true; } + if ( + canMangle === false && + exportInfo.canMangleProvide !== false + ) { + exportInfo.canMangleProvide = false; + changed = true; + } } else { const exportInfo = exportsInfo.getExportInfo( exportNameOrSpec.name @@ -139,11 +147,14 @@ class FlagDependencyExportsPlugin { exportInfo.provided = true; changed = true; } - if (exportNameOrSpec.canMangle === false) { - if (exportInfo.canMangleProvide !== false) { - exportInfo.canMangleProvide = false; - changed = true; - } + if ( + exportInfo.canMangleProvide !== false && + (exportNameOrSpec.canMangle === false || + (canMangle === false && + exportNameOrSpec.canMangle === undefined)) + ) { + exportInfo.canMangleProvide = false; + changed = true; } if (exportNameOrSpec.exports) { const nestedExportsInfo = exportInfo.createNestedExportsInfo(); diff --git a/lib/ModuleGraph.js b/lib/ModuleGraph.js index a115863c1..8df305c63 100644 --- a/lib/ModuleGraph.js +++ b/lib/ModuleGraph.js @@ -70,10 +70,16 @@ class ExportsInfo { this._redirectTo = undefined; } + /** + * @returns {Iterable} all owned exports in any order + */ get ownedExports() { return this._exports.values(); } + /** + * @returns {Iterable} all exports in any order + */ get exports() { if (this._redirectTo) { const map = new Map(this._redirectTo._exports); @@ -85,6 +91,9 @@ class ExportsInfo { return this._exports.values(); } + /** + * @returns {Iterable} all exports in order + */ get orderedExports() { if (!this._exportsAreOrdered) { this._sortExports(); @@ -104,6 +113,9 @@ class ExportsInfo { return this._exports.values(); } + /** + * @returns {ExportInfo} the export info of unlisted exports + */ get otherExportsInfo() { if (this._redirectTo) return this._redirectTo.otherExportsInfo; return this._otherExportsInfo; @@ -223,16 +235,17 @@ class ExportsInfo { } /** + * @param {boolean=} canMangle true, if exports can still be mangled (defaults to false) * @returns {boolean} true, if this call changed something */ - setUnknownExportsProvided() { + setUnknownExportsProvided(canMangle) { let changed = false; for (const exportInfo of this._exports.values()) { if (exportInfo.provided !== true && exportInfo.provided !== null) { exportInfo.provided = null; changed = true; } - if (exportInfo.canMangleProvide !== false) { + if (!canMangle && exportInfo.canMangleProvide !== false) { exportInfo.canMangleProvide = false; changed = true; } @@ -249,7 +262,7 @@ class ExportsInfo { this._otherExportsInfo.provided = null; changed = true; } - if (this._otherExportsInfo.canMangleProvide !== false) { + if (!canMangle && this._otherExportsInfo.canMangleProvide !== false) { this._otherExportsInfo.canMangleProvide = false; changed = true; } diff --git a/lib/dependencies/HarmonyExportImportedSpecifierDependency.js b/lib/dependencies/HarmonyExportImportedSpecifierDependency.js index 130809661..aecc352ed 100644 --- a/lib/dependencies/HarmonyExportImportedSpecifierDependency.js +++ b/lib/dependencies/HarmonyExportImportedSpecifierDependency.js @@ -466,6 +466,7 @@ class HarmonyExportImportedSpecifierDependency extends HarmonyImportDependency { case "dynamic-reexport": return { exports: true, + canMangle: false, // TODO: consider passing `ignored` from `dynamic-reexport` dependencies: [moduleGraph.getModule(this)] }; diff --git a/lib/dependencies/StaticExportsDependency.js b/lib/dependencies/StaticExportsDependency.js index 92534df55..527d6e3c3 100644 --- a/lib/dependencies/StaticExportsDependency.js +++ b/lib/dependencies/StaticExportsDependency.js @@ -35,20 +35,11 @@ class StaticExportsDependency extends NullDependency { * @returns {ExportsSpec | undefined} export names */ getExports(moduleGraph) { - if (!this.canMangle && this.exports !== true) { - return { - exports: this.exports.map(name => ({ - name, - canMangle: false - })), - dependencies: undefined - }; - } else { - return { - exports: this.exports, - dependencies: undefined - }; - } + return { + exports: this.exports, + canMangle: this.canMangle, + dependencies: undefined + }; } /** diff --git a/lib/library/SystemLibraryPlugin.js b/lib/library/SystemLibraryPlugin.js index 653b698fb..0305b2e59 100644 --- a/lib/library/SystemLibraryPlugin.js +++ b/lib/library/SystemLibraryPlugin.js @@ -7,7 +7,9 @@ const { ConcatSource } = require("webpack-sources"); const ExternalModule = require("../ExternalModule"); +const { UsageState } = require("../ModuleGraph"); const Template = require("../Template"); +const propertyAccess = require("../util/propertyAccess"); const AbstractLibraryPlugin = require("./AbstractLibraryPlugin"); /** @typedef {import("webpack-sources").Source} Source */ @@ -67,7 +69,7 @@ class SystemLibraryPlugin extends AbstractLibraryPlugin { * @param {LibraryContext} libraryContext context * @returns {Source} source with library export */ - render(source, { chunkGraph, chunk }, { options, compilation }) { + render(source, { chunkGraph, moduleGraph, chunk }, { options, compilation }) { const modules = chunkGraph .getChunkModules(chunk) .filter(m => m instanceof ExternalModule); @@ -99,10 +101,12 @@ class SystemLibraryPlugin extends AbstractLibraryPlugin { ); // Declaring variables for the internal variable names for the webpack externals - const externalVarDeclarations = - externalWebpackNames.length > 0 - ? `var ${externalWebpackNames.join(", ")};` - : ""; + const externalVarDeclarations = externalWebpackNames + .map(name => `var ${name} = {};`) + .join("\n"); + + // Define __esModule flag on all internal variables and helpers + const externalVarInitialization = []; // The system.register format requires an array of setter functions for externals. const setters = @@ -111,24 +115,65 @@ class SystemLibraryPlugin extends AbstractLibraryPlugin { : Template.asString([ "setters: [", Template.indent( - externalWebpackNames - .map(external => - Template.asString([ + externals + .map((module, i) => { + const external = externalWebpackNames[i]; + const exportsInfo = moduleGraph.getExportsInfo(module); + const otherUnused = + exportsInfo.otherExportsInfo.used === UsageState.Unused; + const instructions = []; + const handledNames = []; + for (const exportInfo of exportsInfo.orderedExports) { + const used = exportInfo.getUsedName(); + if (used) { + if (otherUnused || used !== exportInfo.name) { + instructions.push( + `${external}${propertyAccess([ + used + ])} = module${propertyAccess([exportInfo.name])};` + ); + handledNames.push(exportInfo.name); + } + } else { + handledNames.push(exportInfo.name); + } + } + if (!otherUnused) { + externalVarInitialization.push( + `Object.defineProperty(${external}, "__esModule", { value: true });` + ); + if (handledNames.length > 0) { + const name = `${external}handledNames`; + externalVarInitialization.push( + `var ${name} = ${JSON.stringify(handledNames)};` + ); + instructions.push( + Template.asString([ + "Object.keys(module).forEach(function(key) {", + Template.indent([ + `if(${name}.indexOf(key) >= 0)`, + Template.indent(`${external}[key] = module[key];`) + ]), + "});" + ]) + ); + } else { + instructions.push( + Template.asString([ + "Object.keys(module).forEach(function(key) {", + Template.indent([`${external}[key] = module[key];`]), + "});" + ]) + ); + } + } + if (instructions.length === 0) return "undefined"; + return Template.asString([ "function(module) {", - Template.indent(`${external} = {__esModule: true};`), - Template.indent([ - "for (var key in module) {", - Template.indent("defineGetter(key, module[key]);"), - "}", - "function defineGetter(key, value) {", - Template.indent([ - `Object.defineProperty(${external}, key, {get: function() {return value;}, enumerable: true});` - ]), - "}" - ]), + Template.indent(instructions), "}" - ]) - ) + ]); + }) .join(",\n") ), "]," @@ -139,23 +184,25 @@ class SystemLibraryPlugin extends AbstractLibraryPlugin { `System.register(${name}${systemDependencies}, function(${dynamicExport}) {`, Template.indent([ externalVarDeclarations, + Template.asString(externalVarInitialization), "return {", Template.indent([ setters, "execute: function() {", Template.indent(`${dynamicExport}(`) ]) - ]) - ]) + "\n", + ]), + "" + ]), source, - "\n" + - Template.asString([ - Template.indent([ - Template.indent([Template.indent([");"]), "}"]), - "};" - ]), - "})" - ]) + Template.asString([ + "", + Template.indent([ + Template.indent([Template.indent([");"]), "}"]), + "};" + ]), + "})" + ]) ); } diff --git a/lib/wasm/WebAssemblyParser.js b/lib/wasm/WebAssemblyParser.js index de4a59eb7..b8281e4c0 100644 --- a/lib/wasm/WebAssemblyParser.js +++ b/lib/wasm/WebAssemblyParser.js @@ -184,7 +184,7 @@ class WebAssemblyParser extends Parser { } }); - state.module.addDependency(new StaticExportsDependency(exports, true)); + state.module.addDependency(new StaticExportsDependency(exports, false)); return state; } diff --git a/test/configCases/externals/externals-system/index.js b/test/configCases/externals/externals-system/index.js index 991dba8c5..b9d868946 100644 --- a/test/configCases/externals/externals-system/index.js +++ b/test/configCases/externals/externals-system/index.js @@ -1,4 +1,5 @@ -import external3Default, { namedThing } from 'external3'; +import external3Default, { namedThing } from "external3"; +import "external4"; /* This test verifies that webpack externals are properly indicated as dependencies to System. * Also that when System provides the external variables to webpack that the variables get plumbed diff --git a/test/configCases/externals/externals-system/test.config.js b/test/configCases/externals/externals-system/test.config.js index 7e4445b06..cb39b895b 100644 --- a/test/configCases/externals/externals-system/test.config.js +++ b/test/configCases/externals/externals-system/test.config.js @@ -4,14 +4,18 @@ module.exports = { beforeExecute: () => { System.init({ external1: { - default: "the external1 value", + default: "the external1 value" }, external2: { - default: "the external2 value", + default: "the external2 value" }, external3: { default: "the external3 default export", namedThing: "the external3 named export" + }, + external4: { + default: "the external4 default export", + namedThing: "the external4 named export" } }); }, diff --git a/test/configCases/externals/externals-system/webpack.config.js b/test/configCases/externals/externals-system/webpack.config.js index 01f561ba6..c333f47c4 100644 --- a/test/configCases/externals/externals-system/webpack.config.js +++ b/test/configCases/externals/externals-system/webpack.config.js @@ -5,6 +5,7 @@ module.exports = { externals: { external1: "external1", external2: "external2", - external3: "external3" + external3: "external3", + external4: "external4" } }; diff --git a/test/helpers/fakeSystem.js b/test/helpers/fakeSystem.js index 2b83c4b93..4a934e439 100644 --- a/test/helpers/fakeSystem.js +++ b/test/helpers/fakeSystem.js @@ -69,8 +69,9 @@ const System = { m.executed = true; for (let i = 0; i < m.deps.length; i++) { const dep = m.deps[i]; + const setters = m.mod.setters[i]; System.ensureExecuted(dep); - m.mod.setters[i](System.registry[dep].exports); + if (setters) setters(System.registry[dep].exports); } m.mod.execute(); }