fix(index): don't crash with dynamic import() (#728)

Author: Diogo Franco

package.json

@@ -83,5 +83,8 @@
       "eslint --fix",
       "git add"
     ]
+  },
+  "jest": {
+    "testEnvironment": "node"
   }
 }

src/index.js

@@ -87,11 +87,21 @@ class ExtractTextPlugin {
     }
   }
 
-  static renderExtractedChunk(chunk) {
+  static renderExtractedChunk(compilation, chunk) {
     const source = new ConcatSource();
 
     for (const chunkModule of chunk.modulesIterable) {
-      let moduleSource = chunkModule.source();
+      let moduleSource = chunkModule.source(
+        compilation.dependencyTemplates,
+        compilation.runtimeTemplate
+      );
+
+      // This module was concatenated by the ModuleConcatenationPlugin; because the pitching loader
+      // only produces commonjs results, at least for now things we want to extract can't be in them.
+      // NOTE: if ESM support is added, _this workaround will break_.
+      if (moduleSource instanceof ConcatSource) {
+        moduleSource = null;
+      }
 
       // Async imports (require.ensure(), import().then) are CachedSource module
       // instances caching a ReplaceSource instance, which breaks the plugin
@@ -102,7 +112,13 @@ class ExtractTextPlugin {
       // it's "__webpack_require__();" statements. Skip it.
       if (moduleSource instanceof CachedSource) {
         if (chunkModule[NS] && chunkModule[NS].content) {
-          moduleSource = new RawSource(chunkModule[NS].content[0][1]);
+          moduleSource = new ConcatSource();
+          if (chunkModule[NS].content.length > 1) {
+            console.error(chunkModule[NS].content);
+          }
+          for (const content of chunkModule[NS].content) {
+            moduleSource.add(new RawSource(content[1]));
+          }
         } else {
           moduleSource = null;
         }
@@ -215,10 +231,12 @@ class ExtractTextPlugin {
               const shouldExtract = !!(
                 options.allChunks || isInitialOrHasNoParents(chunk)
               );
-              // chunk.sortModules();
 
               async.forEach(
-                Array.from(chunk.modulesIterable),
+                Array.from(chunk.modulesIterable).sort(
+                  // NOTE: .index should be .index2 once ESM support is added
+                  (a, b) => a.index - b.index
+                ),
                 (module, moduleCallback) => {
                   // eslint-disable-line no-shadow
                   let meta = module[NS];
@@ -231,6 +249,7 @@ class ExtractTextPlugin {
                     // chunk. See issue #604
                     if (shouldExtract && !wasExtracted) {
                       module[`${NS}/extract`] = shouldExtract; // eslint-disable-line no-path-concat
+
                       return compilation.rebuildModule(module, (err) => {
                         if (err) {
                           compilation.errors.push(err);
@@ -328,6 +347,7 @@ class ExtractTextPlugin {
 
             const chunk = extractedChunk.originalChunk;
             const source = ExtractTextPlugin.renderExtractedChunk(
+              compilation,
               extractedChunk
             );
 

src/loader.js

@@ -140,6 +140,7 @@ export function pitch(request) {
 
         this[NS](text, query);
 
+        // NOTE: converting this to ESM will require changes to renderExtractedChunk
         if (text.locals && typeof resultSource !== 'undefined') {
           resultSource += `\nmodule.exports = ${JSON.stringify(text.locals)};`;
         }

test/__snapshots__/webpack-integration.test.js.snap

@@ -1,5 +1,15 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`Webpack Integration Tests chunk-modules-css-wrong-order 1`] = `
+".block {
+  color: tomato;
+}
+.App {
+  color: black;
+}
+"
+`;
+
 exports[`Webpack Integration Tests chunk-modules-nested-ordered-by-id 1`] = `
 "body {
   margin: 0;
@@ -147,6 +157,17 @@ exports[`Webpack Integration Tests splitted-chunk 1`] = `
 "
 `;
 
+exports[`Webpack Integration Tests splitted-chunk-import 1`] = `
+"a
+"
+`;
+
+exports[`Webpack Integration Tests splitted-chunk-import-allchunks 1`] = `
+"a
+b
+"
+`;
+
 exports[`Webpack Integration Tests splitted-multiple-entries 1`] = `""`;
 
 exports[`Webpack Integration Tests splitted-multiple-entries 2`] = `""`;

test/cases/splitted-chunk-import-allchunks/a.txt

@@ -0,0 +1 @@
+a

test/cases/splitted-chunk-import-allchunks/b.txt

@@ -0,0 +1 @@
+b

test/cases/splitted-chunk-import-allchunks/c.js

@@ -0,0 +1 @@
+import './a.txt'

test/cases/splitted-chunk-import-allchunks/expected/file.css

@@ -0,0 +1,2 @@
+a
+b

test/cases/splitted-chunk-import-allchunks/index.js

@@ -0,0 +1,3 @@
+require('./a.txt');
+import('./b.txt');
+import('./c.js');

test/cases/splitted-chunk-import-allchunks/webpack.config.js

@@ -0,0 +1,11 @@
+import ExtractTextPlugin from '../../../src/index';
+
+module.exports = {
+  entry: './index',
+  plugins: [
+    new ExtractTextPlugin({
+      filename: 'file.css',
+      allChunks: true,
+    }),
+  ],
+};

test/cases/splitted-chunk-import/a.txt

@@ -0,0 +1 @@
+a

test/cases/splitted-chunk-import/b.txt

@@ -0,0 +1 @@
+b

test/cases/splitted-chunk-import/c.js

@@ -0,0 +1 @@
+import './a.txt'

test/cases/splitted-chunk-import/expected/file.css

@@ -0,0 +1 @@
+a

test/cases/splitted-chunk-import/index.js

@@ -0,0 +1,3 @@
+require('./a.txt');
+import('./b.txt');
+import('./c.js');

test/cases/splitted-chunk-import/webpack.config.js

@@ -0,0 +1,11 @@
+import ExtractTextPlugin from '../../../src/index';
+
+module.exports = {
+  entry: './index',
+  plugins: [
+    new ExtractTextPlugin({
+      filename: 'file.css',
+      allChunks: false,
+    }),
+  ],
+};