Skip to content

Commit

Permalink
Merge pull request #5180 from webpack/feature/improve-module-concat-b…
Browse files Browse the repository at this point in the history 
…ailout-messages

Improve ModuleConcatenation bailout messages
  • Loading branch information
@sokra
sokra committed Jul 25, 2017
2 parents 4b12c56 + 32264b8 commit b159ec2
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 21 deletions.
32 changes: 19 additions & 13 deletions lib/optimize/ModuleConcatenationPlugin.js
View file
Expand Up @@ -9,6 +9,10 @@ const ConcatenatedModule = require("./ConcatenatedModule");
const HarmonyExportImportedSpecifierDependency = require("../dependencies/HarmonyExportImportedSpecifierDependency");
const HarmonyCompatibilityDependency = require("../dependencies/HarmonyCompatibilityDependency");

function formatBailoutReason(msg) {
return "ModuleConcatenation bailout: " + msg;
}

class ModuleConcatenationPlugin {
constructor(options) {
if(typeof options !== "object") options = {};
Expand All @@ -24,9 +28,9 @@ class ModuleConcatenationPlugin {
});
const bailoutReasonMap = new Map();

function setBailoutReason(module, prefix, reason) {
function setBailoutReason(module, reason) {
bailoutReasonMap.set(module, reason);
module.optimizationBailout.push(typeof reason === "function" ? (rs) => `${prefix}: ${reason(rs)}` : `${prefix}: ${reason}`);
module.optimizationBailout.push(typeof reason === "function" ? (rs) => formatBailoutReason(reason(rs)) : formatBailoutReason(reason));
}

function getBailoutReason(module, requestShortener) {
Expand All @@ -41,50 +45,52 @@ class ModuleConcatenationPlugin {
for(const module of modules) {
// Only harmony modules are valid for optimization
if(!module.meta || !module.meta.harmonyModule || !module.dependencies.some(d => d instanceof HarmonyCompatibilityDependency)) {
setBailoutReason(module, "Module is not an ECMAScript module");
continue;
}

// Because of variable renaming we can't use modules with eval
if(module.meta && module.meta.hasEval) {
setBailoutReason(module, "ModuleConcatenation", "eval is used in the module");
setBailoutReason(module, "Module uses eval()");
continue;
}

relevantModules.push(module);

// Module must not be the entry points
if(module.getChunks().some(chunk => chunk.entryModule === module)) {
setBailoutReason(module, "ModuleConcatenation (inner)", "module is an entrypoint");
setBailoutReason(module, "Module is an entry point");
continue;
}

// Exports must be known (and not dynamic)
if(!Array.isArray(module.providedExports)) {
setBailoutReason(module, "ModuleConcatenation (inner)", "exports are not known");
setBailoutReason(module, "Module exports are unknown");
continue;
}

// Using dependency variables is not possible as this wraps the code in a function
if(module.variables.length > 0) {
setBailoutReason(module, "ModuleConcatenation (inner)", "dependency variables are used (i. e. ProvidePlugin)");
setBailoutReason(module, "Module uses injected variables (usually caused by the ProvidePlugin)");
continue;
}

// Module must only be used by Harmony Imports
const nonHarmonyReasons = module.reasons.filter(reason => !(reason.dependency instanceof HarmonyImportDependency));
if(nonHarmonyReasons.length > 0) {
const importingModules = new Set(nonHarmonyReasons.map(r => r.module));
setBailoutReason(module, "ModuleConcatenation (inner)", (requestShortener) => {
const names = Array.from(importingModules).map(m => m.readableIdentifier(requestShortener)).sort();
return `module is used with non-harmony imports from ${names.join(", ")}`;
const importingModuleTypes = new Map(Array.from(importingModules).map(m => [m, new Set(nonHarmonyReasons.filter(r => r.module === m).map(r => r.dependency.type).sort())]));
setBailoutReason(module, (requestShortener) => {
const names = Array.from(importingModules).map(m => `${m.readableIdentifier(requestShortener)} (referenced with ${Array.from(importingModuleTypes.get(m)).join(", ")})`).sort();
return `Module is referenced from these modules with unsupported syntax: ${names.join(", ")}`;
});
continue;
}

possibleInners.add(module);
}
// sort by depth
// modules with lower depth are more likly suited as roots
// modules with lower depth are more likely suited as roots
// this improves performance, because modules already selected as inner are skipped
relevantModules.sort((a, b) => {
return a.depth - b.depth;
Expand Down Expand Up @@ -137,11 +143,11 @@ class ModuleConcatenationPlugin {
for(const warning of concatConfiguration.warnings) {
newModule.optimizationBailout.push((requestShortener) => {
const reason = getBailoutReason(warning[0], requestShortener);
const reasonPrefix = reason ? `: ${reason}` : "";
const reasonWithPrefix = reason ? ` (<- ${reason})` : "";
if(warning[0] === warning[1])
return `ModuleConcatenation: Cannot concat with ${warning[0].readableIdentifier(requestShortener)}${reasonPrefix}`;
return formatBailoutReason(`Cannot concat with ${warning[0].readableIdentifier(requestShortener)}${reasonWithPrefix}`);
else
return `ModuleConcatenation: Cannot concat with ${warning[0].readableIdentifier(requestShortener)} because of ${warning[1].readableIdentifier(requestShortener)}${reasonPrefix}`;
return formatBailoutReason(`Cannot concat with ${warning[0].readableIdentifier(requestShortener)} because of ${warning[1].readableIdentifier(requestShortener)}${reasonWithPrefix}`);
});
}
const chunks = concatConfiguration.rootModule.getChunks();
Expand Down
16 changes: 8 additions & 8 deletions test/statsCases/scope-hoisting-multi/expected.txt
View file
Expand Up @@ -18,18 +18,18 @@ Child
Time: Xms
[0] (webpack)/test/statsCases/scope-hoisting-multi/common_lazy_shared.js 25 bytes {0} {1} {2} [built]
[1] (webpack)/test/statsCases/scope-hoisting-multi/vendor.js 25 bytes {5} [built]
ModuleConcatenation (inner): module is an entrypoint
ModuleConcatenation bailout: Module is an entry point
[2] (webpack)/test/statsCases/scope-hoisting-multi/common.js + 1 modules 62 bytes {3} {4} [built]
[3] (webpack)/test/statsCases/scope-hoisting-multi/common_lazy.js 25 bytes {1} {2} [built]
[4] (webpack)/test/statsCases/scope-hoisting-multi/first.js + 1 modules 238 bytes {4} [built]
ModuleConcatenation (inner): module is an entrypoint
ModuleConcatenation: Cannot concat with (webpack)/test/statsCases/scope-hoisting-multi/common.js
ModuleConcatenation: Cannot concat with (webpack)/test/statsCases/scope-hoisting-multi/vendor.js: module is an entrypoint
ModuleConcatenation bailout: Module is an entry point
ModuleConcatenation bailout: Cannot concat with (webpack)/test/statsCases/scope-hoisting-multi/common.js
ModuleConcatenation bailout: Cannot concat with (webpack)/test/statsCases/scope-hoisting-multi/vendor.js (<- Module is an entry point)
[5] (webpack)/test/statsCases/scope-hoisting-multi/lazy_shared.js 31 bytes {0} [built]
ModuleConcatenation (inner): module is used with non-harmony imports from (webpack)/test/statsCases/scope-hoisting-multi/first.js, (webpack)/test/statsCases/scope-hoisting-multi/second.js
ModuleConcatenation bailout: Module is referenced from these modules with unsupported syntax: (webpack)/test/statsCases/scope-hoisting-multi/first.js (referenced with import()), (webpack)/test/statsCases/scope-hoisting-multi/second.js (referenced with import())
[6] (webpack)/test/statsCases/scope-hoisting-multi/second.js 177 bytes {3} [built]
ModuleConcatenation (inner): module is an entrypoint
ModuleConcatenation bailout: Module is an entry point
[7] (webpack)/test/statsCases/scope-hoisting-multi/lazy_second.js 55 bytes {1} [built]
ModuleConcatenation (inner): module is used with non-harmony imports from (webpack)/test/statsCases/scope-hoisting-multi/second.js
ModuleConcatenation bailout: Module is referenced from these modules with unsupported syntax: (webpack)/test/statsCases/scope-hoisting-multi/second.js (referenced with import())
[8] (webpack)/test/statsCases/scope-hoisting-multi/lazy_first.js 55 bytes {2} [built]
ModuleConcatenation (inner): module is used with non-harmony imports from (webpack)/test/statsCases/scope-hoisting-multi/first.js
ModuleConcatenation bailout: Module is referenced from these modules with unsupported syntax: (webpack)/test/statsCases/scope-hoisting-multi/first.js (referenced with import())

0 comments on commit b159ec2

Please sign in to comment.