Merge pull request #2863 from dmichon-msft/module-minifier-mini-css-compat

dmichon-msft · web-flow · commit 7470dbf8d9f0 · 2021-08-17T14:43:43.000-07:00
[module-minifier-plugin] Fix compatibility issue with mini-css-extract-plugin
diff --git a/common/changes/@rushstack/module-minifier-plugin/module-minifier-mini-css-compat_2021-08-17-21-01.json b/common/changes/@rushstack/module-minifier-plugin/module-minifier-mini-css-compat_2021-08-17-21-01.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/module-minifier-plugin",
+      "comment": "Fix compatibility issue with mini-css-extract-plugin and other plugins that introduce non-JavaScript modules and asset types.",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@rushstack/module-minifier-plugin",
+  "email": "dmichon-msft@users.noreply.github.com"
+}
diff --git a/webpack/module-minifier-plugin/src/ModuleMinifierPlugin.ts b/webpack/module-minifier-plugin/src/ModuleMinifierPlugin.ts
@@ -221,22 +221,6 @@ export class ModuleMinifierPlugin implements webpack.Plugin {
           }
         }
 
-        /**
-         * Callback to invoke for a chunk during render to replace the modules with CHUNK_MODULES_TOKEN
-         */
-        function dehydrateAsset(modules: Source, chunk: webpack.compilation.Chunk): Source {
-          for (const mod of chunk.modulesIterable) {
-            if (mod.id === null || !submittedModules.has(mod.id)) {
-              console.error(
-                `Chunk ${chunk.id} failed to render module ${mod.id} for ${(mod as IExtendedModule).resource}`
-              );
-            }
-          }
-
-          // Discard the rendered modules
-          return new RawSource(CHUNK_MODULES_TOKEN);
-        }
-
         const { minifier } = this;
 
         const cleanupMinifier: (() => Promise<void>) | undefined = minifier.ref?.();
@@ -467,7 +451,7 @@ export class ModuleMinifierPlugin implements webpack.Plugin {
                     }
                   );
                 } else {
-                  // Skip minification for all other assets, though the modules still are
+                  // This isn't a JS asset. Don't try to minify the asset wrapper, though if it contains modules, those might still get replaced with minified versions.
                   minifiedAssets.set(assetName, {
                     // Still need to restore ids
                     source: postProcessCode(new ReplaceSource(asset), assetName),
@@ -509,9 +493,32 @@ export class ModuleMinifierPlugin implements webpack.Plugin {
           }
         );
 
-        for (const template of [compilation.chunkTemplate, compilation.mainTemplate]) {
-          (template as unknown as IExtendedChunkTemplate).hooks.modules.tap(TAP_AFTER, dehydrateAsset);
-        }
+        // This function is written twice because the parameter order is not the same between the two hooks
+        (compilation.chunkTemplate as unknown as IExtendedChunkTemplate).hooks.modules.tap(
+          TAP_AFTER,
+          (source: Source, chunk: webpack.compilation.Chunk, moduleTemplate: unknown) => {
+            if (moduleTemplate !== compilation.moduleTemplates.javascript) {
+              // This is not a JavaScript asset
+              return source;
+            }
+
+            // Discard the rendered modules
+            return new RawSource(CHUNK_MODULES_TOKEN);
+          }
+        );
+
+        (compilation.mainTemplate as unknown as IExtendedChunkTemplate).hooks.modules.tap(
+          TAP_AFTER,
+          (source: Source, chunk: webpack.compilation.Chunk, hash: unknown, moduleTemplate: unknown) => {
+            if (moduleTemplate !== compilation.moduleTemplates.javascript) {
+              // This is not a JavaScript asset
+              return source;
+            }
+
+            // Discard the rendered modules
+            return new RawSource(CHUNK_MODULES_TOKEN);
+          }
+        );
       }
     );
   }
diff --git a/webpack/module-minifier-plugin/src/RehydrateAsset.ts b/webpack/module-minifier-plugin/src/RehydrateAsset.ts
@@ -14,11 +14,15 @@ import { IAssetInfo, IModuleMap, IModuleInfo } from './ModuleMinifierPlugin.type
  * @public
  */
 export function rehydrateAsset(asset: IAssetInfo, moduleMap: IModuleMap, banner: string): Source {
-  const { source: assetSource, modules, externalNames } = asset;
+  const { source: assetSource, modules } = asset;
 
   const assetCode: string = assetSource.source() as string;
 
   const tokenIndex: number = assetCode.indexOf(CHUNK_MODULES_TOKEN);
+  if (tokenIndex < 0) {
+    // This is not a JS asset.
+    return handleExternals(assetSource, asset);
+  }
   const suffixStart: number = tokenIndex + CHUNK_MODULES_TOKEN.length;
   const suffix: string = assetCode.slice(suffixStart);
 
@@ -138,11 +142,15 @@ export function rehydrateAsset(asset: IAssetInfo, moduleMap: IModuleMap, banner:
 
   source.add(suffix);
 
-  const cached: CachedSource = new CachedSource(source);
+  return handleExternals(new CachedSource(source), asset);
+}
+
+function handleExternals(source: Source, asset: IAssetInfo): Source {
+  const { externalNames } = asset;
 
   if (externalNames.size) {
-    const replaceSource: ReplaceSource = new ReplaceSource(cached);
-    const code: string = cached.source() as string;
+    const replaceSource: ReplaceSource = new ReplaceSource(source);
+    const code: string = source.source() as string;
 
     const externalIdRegex: RegExp = /__WEBPACK_EXTERNAL_MODULE_[A-Za-z0-9_$]+/g;
 
@@ -162,5 +170,5 @@ export function rehydrateAsset(asset: IAssetInfo, moduleMap: IModuleMap, banner:
     return new CachedSource(replaceSource);
   }
 
-  return cached;
+  return source;
 }
