fix(order): use correct order when multiple chunk groups are merged (#246)

Author: sokra

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

test/cases/split-chunks-single/a.css

@@ -0,0 +1 @@
+body { content: "a"; }

test/cases/split-chunks-single/b.css

@@ -0,0 +1 @@
+body { content: "b"; }

test/cases/split-chunks-single/c.css

@@ -0,0 +1 @@
+body { content: "c"; }

test/cases/split-chunks-single/chunk1.js

@@ -0,0 +1,2 @@
+import "./c.css";
+import "./d.css";

test/cases/split-chunks-single/chunk2.js

@@ -0,0 +1,2 @@
+import "./d.css";
+import "./h.css";

test/cases/split-chunks-single/d.css

@@ -0,0 +1 @@
+body { content: "d"; }

test/cases/split-chunks-single/e1.css

@@ -0,0 +1 @@
+body { content: "e1"; }

test/cases/split-chunks-single/e2.css

@@ -0,0 +1 @@
+body { content: "e2"; }

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");

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';

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"; }
+

test/cases/split-chunks-single/f.css

@@ -0,0 +1 @@
+body { content: "f"; }

test/cases/split-chunks-single/g.css

@@ -0,0 +1 @@
+body { content: "g"; }

test/cases/split-chunks-single/h.css

@@ -0,0 +1 @@
+body { content: "h"; }

test/cases/split-chunks-single/style.css

@@ -0,0 +1 @@
+body { background: red; }

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',
+    }),
+  ],
+};