Skip to content

Commit

Permalink
Merge pull request #8243 from webpack/bugfix/stablility
Browse files Browse the repository at this point in the history
fix some compilation stablility issues
  • Loading branch information
sokra committed Oct 19, 2018
2 parents 1d9f8a3 + 5e4a6a4 commit 07d2d65
Show file tree
Hide file tree
Showing 8 changed files with 1,062 additions and 993 deletions.
4 changes: 4 additions & 0 deletions .travis.yml
Expand Up @@ -32,6 +32,10 @@ matrix:
node_js: "10"
env: NO_WATCH_TESTS=1 JEST="--maxWorkers=2 --cacheDirectory .jest-cache" JOB_PART=integration
stage: advanced
- os: linux
node_js: "10"
env: NO_WATCH_TESTS=1 ALTERNATIVE_SORT=1 JEST="--maxWorkers=2 --cacheDirectory .jest-cache" JOB_PART=integration
stage: versions
- os: osx
node_js: "10"
env: NO_WATCH_TESTS=1 JEST="--maxWorkers=2 --cacheDirectory .jest-cache" JOB_PART=integration
Expand Down
17 changes: 14 additions & 3 deletions lib/Chunk.js
Expand Up @@ -182,12 +182,13 @@ class Chunk {
*/
hasRuntime() {
for (const chunkGroup of this._groups) {
// We only need to check the first one
return (
if (
chunkGroup.isInitial() &&
chunkGroup instanceof Entrypoint &&
chunkGroup.getRuntimeChunk() === this
);
) {
return true;
}
}
return false;
}
Expand Down Expand Up @@ -313,6 +314,10 @@ class Chunk {
* @returns {-1|0|1} this is a comparitor function like sort and returns -1, 0, or 1 based on sort order
*/
compareTo(otherChunk) {
if (this.name && !otherChunk.name) return -1;
if (!this.name && otherChunk.name) return 1;
if (this.name < otherChunk.name) return -1;
if (this.name > otherChunk.name) return 1;
if (this._modules.size > otherChunk._modules.size) return -1;
if (this._modules.size < otherChunk._modules.size) return 1;
this._modules.sort();
Expand Down Expand Up @@ -390,6 +395,10 @@ class Chunk {
}
otherChunk._modules.clear();

if (otherChunk.entryModule) {
this.entryModule = otherChunk.entryModule;
}

for (const chunkGroup of otherChunk._groups) {
chunkGroup.replaceChunk(otherChunk, this);
this.addGroup(chunkGroup);
Expand All @@ -405,6 +414,8 @@ class Chunk {
} else {
this.name = this.name < otherChunk.name ? this.name : otherChunk.name;
}
} else if (otherChunk.name) {
this.name = otherChunk.name;
}

return true;
Expand Down
22 changes: 18 additions & 4 deletions lib/Compilation.js
Expand Up @@ -108,10 +108,11 @@ const compareLocations = require("./compareLocations");
* @returns {-1|0|1} sort value
*/
const byId = (a, b) => {
if (a.id !== null && b.id !== null) {
if (a.id < b.id) return -1;
if (a.id > b.id) return 1;
if (typeof a.id !== typeof b.id) {
return typeof a.id < typeof b.id ? -1 : 1;
}
if (a.id < b.id) return -1;
if (a.id > b.id) return 1;
return 0;
};

Expand All @@ -121,6 +122,9 @@ const byId = (a, b) => {
* @returns {-1|0|1} sort value
*/
const byIdOrIdentifier = (a, b) => {
if (typeof a.id !== typeof b.id) {
return typeof a.id < typeof b.id ? -1 : 1;
}
if (a.id < b.id) return -1;
if (a.id > b.id) return 1;
const identA = a.identifier();
Expand Down Expand Up @@ -645,7 +649,15 @@ class Compilation extends Tapable {
war.dependencies = dependencies;
this.warnings.push(war);
}
module.dependencies.sort((a, b) => compareLocations(a.loc, b.loc));
const originalMap = module.dependencies.reduce((map, v, i) => {
map.set(v, i);
return map;
}, new Map());
module.dependencies.sort((a, b) => {
const cmp = compareLocations(a.loc, b.loc);
if (cmp) return cmp;
return originalMap.get(a) - originalMap.get(b);
});
if (error) {
this.hooks.failedModule.call(module, error);
return callback(error);
Expand Down Expand Up @@ -2127,6 +2139,8 @@ class Compilation extends Tapable {
for (let indexChunk = 0; indexChunk < chunks.length; indexChunk++) {
chunks[indexChunk].sortItems();
}

chunks.sort((a, b) => a.compareTo(b));
}

sortItemsWithChunkIds() {
Expand Down
10 changes: 10 additions & 0 deletions lib/Entrypoint.js
Expand Up @@ -49,6 +49,16 @@ class Entrypoint extends ChunkGroup {
getRuntimeChunk() {
return this.runtimeChunk || this.chunks[0];
}

/**
* @param {Chunk} oldChunk chunk to be replaced
* @param {Chunk} newChunk New chunkt that will be replaced
* @returns {boolean} rerturns true for
*/
replaceChunk(oldChunk, newChunk) {
if (this.runtimeChunk === oldChunk) this.runtimeChunk = newChunk;
return super.replaceChunk(oldChunk, newChunk);
}
}

module.exports = Entrypoint;
87 changes: 42 additions & 45 deletions lib/Stats.js
Expand Up @@ -17,6 +17,9 @@ const optionsOrFallback = (...args) => {
};

const compareId = (a, b) => {
if (typeof a !== typeof b) {
return typeof a < typeof b ? -1 : 1;
}
if (a < b) return -1;
if (a > b) return 1;
return 0;
Expand Down Expand Up @@ -250,21 +253,27 @@ class Stats {
return a[fieldKey] < b[fieldKey] ? -1 : 1;
};

const sortByField = field => (a, b) => {
if (!field) {
return 0;
}

const fieldKey = this.normalizeFieldKey(field);

// if a field is prefixed with a "!" the sort is reversed!
const sortIsRegular = this.sortOrderRegular(field);

return sortByFieldAndOrder(
fieldKey,
sortIsRegular ? a : b,
sortIsRegular ? b : a
);
const sortByField = (field, originalArray) => {
const originalMap = originalArray.reduce((map, v, i) => {
map.set(v, i);
return map;
}, new Map());
return (a, b) => {
if (field) {
const fieldKey = this.normalizeFieldKey(field);

// if a field is prefixed with a "!" the sort is reversed!
const sortIsRegular = this.sortOrderRegular(field);

const cmp = sortByFieldAndOrder(
fieldKey,
sortIsRegular ? a : b,
sortIsRegular ? b : a
);
if (cmp) return cmp;
}
return originalMap.get(a) - originalMap.get(b);
};
};

const formatError = e => {
Expand Down Expand Up @@ -380,7 +389,7 @@ class Stats {
}
if (showAssets) {
const assetsByFile = {};
const compilationAssets = Object.keys(compilation.assets);
const compilationAssets = Object.keys(compilation.assets).sort();
obj.assetsByChunkName = {};
obj.assets = compilationAssets
.map(asset => {
Expand Down Expand Up @@ -421,7 +430,7 @@ class Stats {
}
}
}
obj.assets.sort(sortByField(sortAssets));
obj.assets.sort(sortByField(sortAssets, obj.assets));
}

const fnChunkGroup = groupMap => {
Expand Down Expand Up @@ -589,11 +598,11 @@ class Stats {
if (module.modules) {
const modules = module.modules;
obj.modules = modules
.sort(sortByField("depth"))
.sort(sortByField("depth", modules))
.filter(createModuleFilter())
.map(fnModule);
obj.filteredModules = modules.length - obj.modules.length;
obj.modules.sort(sortByField(sortModules));
obj.modules.sort(sortByField(sortModules, obj.modules));
}
}
if (showSource && module._source) {
Expand Down Expand Up @@ -639,13 +648,14 @@ class Stats {
childrenByOrder: childIdByOrder
};
if (showChunkModules) {
obj.modules = chunk
.getModules()
.sort(sortByField("depth"))
const modules = chunk.getModules();
obj.modules = modules
.slice()
.sort(sortByField("depth", modules))
.filter(createModuleFilter())
.map(fnModule);
obj.filteredModules = chunk.getNumberOfModules() - obj.modules.length;
obj.modules.sort(sortByField(sortModules));
obj.modules.sort(sortByField(sortModules, obj.modules));
}
if (showChunkOrigins) {
obj.origins = Array.from(chunk.groupsIterable, g => g.origins)
Expand All @@ -662,40 +672,27 @@ class Stats {
reasons: origin.reasons || []
}))
.sort((a, b) => {
if (
typeof a.moduleId === "number" &&
typeof b.moduleId !== "number"
)
return 1;
if (
typeof a.moduleId !== "number" &&
typeof b.moduleId === "number"
)
return -1;
if (
typeof a.moduleId === "number" &&
typeof b.moduleId === "number"
) {
const diffId = a.moduleId - b.moduleId;
if (diffId !== 0) return diffId;
}
if (a.loc < b.loc) return -1;
if (a.loc > b.loc) return 1;
const cmp1 = compareId(a.moduleId, b.moduleId);
if (cmp1) return cmp1;
const cmp2 = compareId(a.loc, b.loc);
if (cmp2) return cmp2;
const cmp3 = compareId(a.request, b.request);
if (cmp3) return cmp3;
return 0;
});
}
return obj;
});
obj.chunks.sort(sortByField(sortChunks));
obj.chunks.sort(sortByField(sortChunks, obj.chunks));
}
if (showModules) {
obj.modules = compilation.modules
.slice()
.sort(sortByField("depth"))
.sort(sortByField("depth", compilation.modules))
.filter(createModuleFilter())
.map(fnModule);
obj.filteredModules = compilation.modules.length - obj.modules.length;
obj.modules.sort(sortByField(sortModules));
obj.modules.sort(sortByField(sortModules, obj.modules));
}
if (showChildren) {
obj.children = compilation.children.map((child, idx) => {
Expand Down
16 changes: 11 additions & 5 deletions lib/optimize/LimitChunkCountPlugin.js
Expand Up @@ -30,11 +30,13 @@ class LimitChunkCountPlugin {
if (maxChunks < 1) return;
if (chunks.length <= maxChunks) return;

const sortedExtendedPairCombinations = chunks
const orderedChunks = chunks.slice().sort((a, b) => a.compareTo(b));

const sortedExtendedPairCombinations = orderedChunks
.reduce((combinations, a, idx) => {
// create combination pairs
for (let i = 0; i < idx; i++) {
const b = chunks[i];
const b = orderedChunks[i];
combinations.push([b, a]);
}
return combinations;
Expand All @@ -54,9 +56,13 @@ class LimitChunkCountPlugin {
.sort((a, b) => {
// sadly javascript does an inplace sort here
// sort them by size
const diff = b[0] - a[0];
if (diff !== 0) return diff;
return a[1] - b[1];
const diff1 = b[0] - a[0];
if (diff1 !== 0) return diff1;
const diff2 = a[1] - b[1];
if (diff2 !== 0) return diff2;
const diff3 = a[2].compareTo(b[2]);
if (diff3 !== 0) return diff3;
return a[3].compareTo(b[3]);
});

const pair = sortedExtendedPairCombinations[0];
Expand Down

0 comments on commit 07d2d65

Please sign in to comment.