Skip to content

Commit

Permalink
fix(order): use correct order when multiple chunk groups are merged (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
sokra authored and evilebottnawi committed Aug 21, 2018
1 parent 3d12bc7 commit c3b363d
Show file tree
Hide file tree
Showing 17 changed files with 167 additions and 10 deletions.
98 changes: 88 additions & 10 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ class MiniCssExtractPlugin {
result.push({
render: () =>
this.renderContentAsset(
compilation,
chunk,
renderedModules,
compilation.runtimeTemplate.requestShortener
Expand All @@ -192,6 +193,7 @@ class MiniCssExtractPlugin {
result.push({
render: () =>
this.renderContentAsset(
compilation,
chunk,
renderedModules,
compilation.runtimeTemplate.requestShortener
Expand Down Expand Up @@ -381,27 +383,103 @@ class MiniCssExtractPlugin {
return obj;
}

renderContentAsset(chunk, modules, requestShortener) {
// get first chunk group and take ordr from this one
// When a chunk is shared between multiple chunk groups
// with different order this can lead to wrong order
// but it's not possible to create a correct order in
// this case. Don't share chunks if you don't like it.
renderContentAsset(compilation, chunk, modules, requestShortener) {
let usedModules;

const [chunkGroup] = chunk.groupsIterable;
if (typeof chunkGroup.getModuleIndex2 === 'function') {
modules.sort(
(a, b) => chunkGroup.getModuleIndex2(a) - chunkGroup.getModuleIndex2(b)
);
// Store dependencies for modules
const moduleDependencies = new Map(modules.map((m) => [m, new Set()]));

// Get ordered list of modules per chunk group
// This loop also gathers dependencies from the ordered lists
// Lists are in reverse order to allow to use Array.pop()
const modulesByChunkGroup = Array.from(chunk.groupsIterable, (cg) => {
const sortedModules = modules
.map((m) => {
return {
module: m,
index: cg.getModuleIndex2(m),
};
})
.filter((item) => item.index !== undefined)
.sort((a, b) => b.index - a.index)
.map((item) => item.module);
for (let i = 0; i < sortedModules.length; i++) {
const set = moduleDependencies.get(sortedModules[i]);
for (let j = i + 1; j < sortedModules.length; j++) {
set.add(sortedModules[j]);
}
}

return sortedModules;
});

// set with already included modules in correct order
usedModules = new Set();

const unusedModulesFilter = (m) => !usedModules.has(m);

while (usedModules.size < modules.length) {
let success = false;
let bestMatch;
let bestMatchDeps;
// get first module where dependencies are fulfilled
for (const list of modulesByChunkGroup) {
// skip and remove already added modules
while (list.length > 0 && usedModules.has(list[list.length - 1]))
list.pop();

// skip empty lists
if (list.length !== 0) {
const module = list[list.length - 1];
const deps = moduleDependencies.get(module);
// determine dependencies that are not yet included
const failedDeps = Array.from(deps).filter(unusedModulesFilter);

// store best match for fallback behavior
if (!bestMatchDeps || bestMatchDeps.length > failedDeps.length) {
bestMatch = list;
bestMatchDeps = failedDeps;
}
if (failedDeps.length === 0) {
// use this module and remove it from list
usedModules.add(list.pop());
success = true;
break;
}
}
}

if (!success) {
// no module found => there is a conflict
// use list with fewest failed deps
// and emit a warning
const fallbackModule = bestMatch.pop();
compilation.warnings.push(
new Error(
`chunk ${chunk.name || chunk.id} [mini-css-extract-plugin]\n` +
'Conflicting order between:\n' +

This comment has been minimized.

Copy link
@jsg2021

jsg2021 Aug 22, 2018

What does this mean? How do we resolve these errors?

` * ${fallbackModule.readableIdentifier(requestShortener)}\n` +
`${bestMatchDeps
.map((m) => ` * ${m.readableIdentifier(requestShortener)}`)
.join('\n')}`
)
);
usedModules.add(fallbackModule);
}
}
} else {
// fallback for older webpack versions
// (to avoid a breaking change)
// TODO remove this in next mayor version
// and increase minimum webpack version to 4.12.0
modules.sort((a, b) => a.index2 - b.index2);
usedModules = modules;
}
const source = new ConcatSource();
const externalsSource = new ConcatSource();
for (const m of modules) {
for (const m of usedModules) {
if (/^@import url/.test(m.content)) {
// HACK for IE
// http://stackoverflow.com/a/14676665/1458162
Expand Down
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/a.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "a"; }
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/b.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "b"; }
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/c.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "c"; }
2 changes: 2 additions & 0 deletions test/cases/split-chunks-single/chunk1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import "./c.css";
import "./d.css";
2 changes: 2 additions & 0 deletions test/cases/split-chunks-single/chunk2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import "./d.css";
import "./h.css";
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/d.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "d"; }
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/e1.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "e1"; }
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/e2.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "e2"; }
6 changes: 6 additions & 0 deletions test/cases/split-chunks-single/entry1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import './a.css';
import './e1.css';
import './e2.css';
import './f.css';
import("./chunk1");
import("./chunk2");
5 changes: 5 additions & 0 deletions test/cases/split-chunks-single/entry2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import './b.css';
import './e2.css';
import './e1.css';
import './g.css';
import './h.css';
18 changes: 18 additions & 0 deletions test/cases/split-chunks-single/expected/styles.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
body { content: "a"; }

This comment has been minimized.

Copy link
@tomlagier

tomlagier Oct 3, 2018

We are running into issues with our load order, (it seems to have changed from 0.4.0 to 0.4.3) and we think it is related to this commit. I'm having a hard time understanding how this test case functions. Is there any chance someone could help explain how the input entries result in this output?


body { content: "b"; }

body { content: "c"; }

body { content: "d"; }

body { content: "e1"; }

body { content: "e2"; }

body { content: "f"; }

body { content: "g"; }

body { content: "h"; }

1 change: 1 addition & 0 deletions test/cases/split-chunks-single/f.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "f"; }
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/g.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "g"; }
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/h.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { content: "h"; }
1 change: 1 addition & 0 deletions test/cases/split-chunks-single/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { background: red; }
36 changes: 36 additions & 0 deletions test/cases/split-chunks-single/webpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
const Self = require('../../../');

module.exports = {
entry: {
entry1: './entry1.js',
entry2: './entry2.js'
},
module: {
rules: [
{
test: /\.css$/,
use: [
Self.loader,
'css-loader',
],
},
],
},
optimization: {
splitChunks: {
cacheGroups: {
styles: {
name: "styles",
chunks: 'all',
test: /\.css$/,
enforce: true
}
}
}
},
plugins: [
new Self({
filename: '[name].css',
}),
],
};

0 comments on commit c3b363d

Please sign in to comment.