From c3b363df41ff15abd7e356327473b4e749009ef0 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 21 Aug 2018 15:10:56 +0200 Subject: [PATCH] fix(order): use correct order when multiple chunk groups are merged (#246) --- src/index.js | 98 +++++++++++++++++-- test/cases/split-chunks-single/a.css | 1 + test/cases/split-chunks-single/b.css | 1 + test/cases/split-chunks-single/c.css | 1 + test/cases/split-chunks-single/chunk1.js | 2 + test/cases/split-chunks-single/chunk2.js | 2 + test/cases/split-chunks-single/d.css | 1 + test/cases/split-chunks-single/e1.css | 1 + test/cases/split-chunks-single/e2.css | 1 + test/cases/split-chunks-single/entry1.js | 6 ++ test/cases/split-chunks-single/entry2.js | 5 + .../split-chunks-single/expected/styles.css | 18 ++++ test/cases/split-chunks-single/f.css | 1 + test/cases/split-chunks-single/g.css | 1 + test/cases/split-chunks-single/h.css | 1 + test/cases/split-chunks-single/style.css | 1 + .../split-chunks-single/webpack.config.js | 36 +++++++ 17 files changed, 167 insertions(+), 10 deletions(-) create mode 100644 test/cases/split-chunks-single/a.css create mode 100644 test/cases/split-chunks-single/b.css create mode 100644 test/cases/split-chunks-single/c.css create mode 100644 test/cases/split-chunks-single/chunk1.js create mode 100644 test/cases/split-chunks-single/chunk2.js create mode 100644 test/cases/split-chunks-single/d.css create mode 100644 test/cases/split-chunks-single/e1.css create mode 100644 test/cases/split-chunks-single/e2.css create mode 100644 test/cases/split-chunks-single/entry1.js create mode 100644 test/cases/split-chunks-single/entry2.js create mode 100644 test/cases/split-chunks-single/expected/styles.css create mode 100644 test/cases/split-chunks-single/f.css create mode 100644 test/cases/split-chunks-single/g.css create mode 100644 test/cases/split-chunks-single/h.css create mode 100644 test/cases/split-chunks-single/style.css create mode 100644 test/cases/split-chunks-single/webpack.config.js diff --git a/src/index.js b/src/index.js index b266b331..c8f641bb 100644 --- a/src/index.js +++ b/src/index.js @@ -167,6 +167,7 @@ class MiniCssExtractPlugin { result.push({ render: () => this.renderContentAsset( + compilation, chunk, renderedModules, compilation.runtimeTemplate.requestShortener @@ -192,6 +193,7 @@ class MiniCssExtractPlugin { result.push({ render: () => this.renderContentAsset( + compilation, chunk, renderedModules, compilation.runtimeTemplate.requestShortener @@ -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' + + ` * ${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 diff --git a/test/cases/split-chunks-single/a.css b/test/cases/split-chunks-single/a.css new file mode 100644 index 00000000..dd4f97ce --- /dev/null +++ b/test/cases/split-chunks-single/a.css @@ -0,0 +1 @@ +body { content: "a"; } diff --git a/test/cases/split-chunks-single/b.css b/test/cases/split-chunks-single/b.css new file mode 100644 index 00000000..52d18453 --- /dev/null +++ b/test/cases/split-chunks-single/b.css @@ -0,0 +1 @@ +body { content: "b"; } diff --git a/test/cases/split-chunks-single/c.css b/test/cases/split-chunks-single/c.css new file mode 100644 index 00000000..c53028fb --- /dev/null +++ b/test/cases/split-chunks-single/c.css @@ -0,0 +1 @@ +body { content: "c"; } diff --git a/test/cases/split-chunks-single/chunk1.js b/test/cases/split-chunks-single/chunk1.js new file mode 100644 index 00000000..eb558470 --- /dev/null +++ b/test/cases/split-chunks-single/chunk1.js @@ -0,0 +1,2 @@ +import "./c.css"; +import "./d.css"; diff --git a/test/cases/split-chunks-single/chunk2.js b/test/cases/split-chunks-single/chunk2.js new file mode 100644 index 00000000..7254062a --- /dev/null +++ b/test/cases/split-chunks-single/chunk2.js @@ -0,0 +1,2 @@ +import "./d.css"; +import "./h.css"; diff --git a/test/cases/split-chunks-single/d.css b/test/cases/split-chunks-single/d.css new file mode 100644 index 00000000..1b2fc6a8 --- /dev/null +++ b/test/cases/split-chunks-single/d.css @@ -0,0 +1 @@ +body { content: "d"; } diff --git a/test/cases/split-chunks-single/e1.css b/test/cases/split-chunks-single/e1.css new file mode 100644 index 00000000..9028097f --- /dev/null +++ b/test/cases/split-chunks-single/e1.css @@ -0,0 +1 @@ +body { content: "e1"; } diff --git a/test/cases/split-chunks-single/e2.css b/test/cases/split-chunks-single/e2.css new file mode 100644 index 00000000..af21d863 --- /dev/null +++ b/test/cases/split-chunks-single/e2.css @@ -0,0 +1 @@ +body { content: "e2"; } diff --git a/test/cases/split-chunks-single/entry1.js b/test/cases/split-chunks-single/entry1.js new file mode 100644 index 00000000..db2fccba --- /dev/null +++ b/test/cases/split-chunks-single/entry1.js @@ -0,0 +1,6 @@ +import './a.css'; +import './e1.css'; +import './e2.css'; +import './f.css'; +import("./chunk1"); +import("./chunk2"); diff --git a/test/cases/split-chunks-single/entry2.js b/test/cases/split-chunks-single/entry2.js new file mode 100644 index 00000000..a8576c43 --- /dev/null +++ b/test/cases/split-chunks-single/entry2.js @@ -0,0 +1,5 @@ +import './b.css'; +import './e2.css'; +import './e1.css'; +import './g.css'; +import './h.css'; diff --git a/test/cases/split-chunks-single/expected/styles.css b/test/cases/split-chunks-single/expected/styles.css new file mode 100644 index 00000000..1bd8b96d --- /dev/null +++ b/test/cases/split-chunks-single/expected/styles.css @@ -0,0 +1,18 @@ +body { content: "a"; } + +body { content: "b"; } + +body { content: "c"; } + +body { content: "d"; } + +body { content: "e1"; } + +body { content: "e2"; } + +body { content: "f"; } + +body { content: "g"; } + +body { content: "h"; } + diff --git a/test/cases/split-chunks-single/f.css b/test/cases/split-chunks-single/f.css new file mode 100644 index 00000000..08f73240 --- /dev/null +++ b/test/cases/split-chunks-single/f.css @@ -0,0 +1 @@ +body { content: "f"; } diff --git a/test/cases/split-chunks-single/g.css b/test/cases/split-chunks-single/g.css new file mode 100644 index 00000000..098e2a66 --- /dev/null +++ b/test/cases/split-chunks-single/g.css @@ -0,0 +1 @@ +body { content: "g"; } diff --git a/test/cases/split-chunks-single/h.css b/test/cases/split-chunks-single/h.css new file mode 100644 index 00000000..991f82ec --- /dev/null +++ b/test/cases/split-chunks-single/h.css @@ -0,0 +1 @@ +body { content: "h"; } diff --git a/test/cases/split-chunks-single/style.css b/test/cases/split-chunks-single/style.css new file mode 100644 index 00000000..31fc5b8a --- /dev/null +++ b/test/cases/split-chunks-single/style.css @@ -0,0 +1 @@ +body { background: red; } diff --git a/test/cases/split-chunks-single/webpack.config.js b/test/cases/split-chunks-single/webpack.config.js new file mode 100644 index 00000000..9732c2a0 --- /dev/null +++ b/test/cases/split-chunks-single/webpack.config.js @@ -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', + }), + ], +};