Repository: microsoft/griffel
Pull request #356 (2023-03-27 09:46:36)

chore(extraction-plugin): redo chunk attachment
This PR redoes chunk management in the plugin to prevent "phantom chunks" that we were creating before.

- Griffel chunk will be created only if needed
- A hack with `GriffelDummyModule` was removed
- We will reuse `SplitChunksPlugin` if it's enabled to avoid redundant module movements
- New fixture added for `optiminizatin.splitChunks = false`

github-actions[bot]: ## 📊 Bundle size report

<sub>🤖 This report was generated against <a href='https://github.com/microsoft/griffel/commit/9fb785b6488a4cbb6dcd135f9ccec55bf260ec4d'>9fb785b6488a4cbb6dcd135f9ccec55bf260ec4d</a></sub>

<!-- Sticky Pull Request Comment -->

Patch:
diff --git a/change/@griffel-webpack-extraction-plugin-0a0405f4-b905-404d-82fb-ef124df6b3f9.json b/change/@griffel-webpack-extraction-plugin-0a0405f4-b905-404d-82fb-ef124df6b3f9.json
new file mode 100644
index 0000000..ca4f178
--- /dev/null
+++ b/change/@griffel-webpack-extraction-plugin-0a0405f4-b905-404d-82fb-ef124df6b3f9.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "chore(extraction-plugin): redo chunk attachment",
+  "packageName": "@griffel/webpack-extraction-plugin",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/code.ts b/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/code.ts
new file mode 100644
index 0000000..19dad50
--- /dev/null
+++ b/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/code.ts
@@ -0,0 +1,22 @@
+import { __styles } from '@griffel/react';
+
+export const styles = __styles(
+  {
+    root: {
+      sj55zd: 'fe3e8s9',
+      uwmqm3: ['fycuoez', 'f8wuabp'],
+    },
+    icon: {
+      De3pzq: 'fcnqdeg',
+      Bi91k9c: 'faf35ka',
+    },
+  },
+  {
+    d: [
+      '.fe3e8s9{color:red;}',
+      '.fycuoez{padding-left:4px;}',
+      '.f8wuabp{padding-right:4px;}',
+      '.fcnqdeg{background-color:green;}',
+    ],
+  },
+);
diff --git a/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/fs.json b/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/fs.json
new file mode 100644
index 0000000..966fca3
--- /dev/null
+++ b/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/fs.json
@@ -0,0 +1,1 @@
+["bundle.js", "griffel.css"]
diff --git a/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/output.css b/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/output.css
new file mode 100644
index 0000000..f1412ed
--- /dev/null
+++ b/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/output.css
@@ -0,0 +1,13 @@
+/** griffel.css **/
+.fe3e8s9 {
+  color: red;
+}
+.fycuoez {
+  padding-left: 4px;
+}
+.f8wuabp {
+  padding-right: 4px;
+}
+.fcnqdeg {
+  background-color: green;
+}
diff --git a/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/output.ts b/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/output.ts
new file mode 100644
index 0000000..fd6229b
--- /dev/null
+++ b/packages/webpack-extraction-plugin/__fixtures__/webpack/config-no-split-chunks/output.ts
@@ -0,0 +1,14 @@
+import { __css as _css } from '@griffel/react';
+import { __styles } from '@griffel/react';
+export const styles = _css({
+  root: {
+    sj55zd: 'fe3e8s9',
+    uwmqm3: ['fycuoez', 'f8wuabp'],
+  },
+  icon: {
+    De3pzq: 'fcnqdeg',
+    Bi91k9c: 'faf35ka',
+  },
+});
+
+require('./code.griffel.css!=!../../../virtual-loader/index.js?style=%2F**%20%40griffel%3Acss-start%20%5Bd%5D%20**%2F%0A.fe3e8s9%7Bcolor%3Ared%3B%7D.fycuoez%7Bpadding-left%3A4px%3B%7D.f8wuabp%7Bpadding-right%3A4px%3B%7D.fcnqdeg%7Bbackground-color%3Agreen%3B%7D%0A%2F**%20%40griffel%3Acss-end%20**%2F%0A!./code.ts');
diff --git a/packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.ts b/packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.ts
index e6a6a07..dd40ce6 100644
--- a/packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.ts
+++ b/packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.ts
@@ -2,13 +2,12 @@ import { defaultCompareMediaQueries, GriffelRenderer } from '@griffel/core';
 import { Compilation } from 'webpack';
 import type { Chunk, Compiler, Module, sources } from 'webpack';
 
-import { GriffelDummyModule } from './GriffelDummyModule';
 import { parseCSSRules } from './parseCSSRules';
 import { sortCSSRules } from './sortCSSRules';
 
 // Webpack does not export these constants
 // https://github.com/webpack/webpack/blob/b67626c7b4ffed8737d195b27c8cea1e68d58134/lib/OptimizationStages.js#L8
-const OPTIMIZE_CHUNKS_STAGE_BASIC = -10;
+const OPTIMIZE_CHUNKS_STAGE_ADVANCED = 10;
 
 export type GriffelCSSExtractionPluginOptions = {
   compareMediaQueries?: GriffelRenderer['compareMediaQueries'];
@@ -26,13 +25,8 @@ function attachGriffelChunkToMainEntryPoint(compilation: Compilation, griffelChu
 
   const mainEntryPoint = entryPoints[0];
   const targetChunk = mainEntryPoint.getEntrypointChunk();
-  const targetChunkGroup = Array.from(targetChunk.groupsIterable)[0];
 
-  mainEntryPoint.pushChunk(griffelChunk);
-
-  // It's mandatory to have the chunk in a group, otherwise ".groupsIterable" will be empty and will fail mini-css-extract
-  // https://github.com/webpack-contrib/mini-css-extract-plugin/blob/26334462e419026086856787d672b052cd916c62/src/index.js#L1125-L1130
-  griffelChunk.addGroup(targetChunkGroup);
+  targetChunk.split(griffelChunk);
 }
 
 function getAssetSourceContents(assetSource: sources.Source): string {
@@ -66,11 +60,11 @@ function isGriffelCSSModule(module: Module): boolean {
   return false;
 }
 
-function moveCSSModulesToGriffelChunk(compilation: Compilation, attachToMainEntryPoint: boolean) {
-  const griffelChunk = compilation.namedChunks.get('griffel');
+function moveCSSModulesToGriffelChunk(compilation: Compilation) {
+  let griffelChunk = compilation.namedChunks.get('griffel');
 
   if (!griffelChunk) {
-    return;
+    griffelChunk = compilation.addChunk('griffel');
   }
 
   const matchingChunks = new Set<Chunk>();
@@ -94,10 +88,8 @@ function moveCSSModulesToGriffelChunk(compilation: Compilation, attachToMainEntr
     }
   }
 
-  if (!attachToMainEntryPoint) {
-    for (const chunk of matchingChunks) {
-      chunk.split(griffelChunk);
-    }
+  for (const chunk of matchingChunks) {
+    chunk.split(griffelChunk);
   }
 }
 
@@ -115,48 +107,62 @@ export class GriffelCSSExtractionPlugin {
   }
 
   apply(compiler: Compiler): void {
-    compiler.hooks.compilation.tap(PLUGIN_NAME, compilation => {
-      // A chunk where we will move all CSS modules
-      const griffelChunk = compilation.addChunk('griffel');
-      // Set up a dummy module to prevent the chunk from getting cleaned up by RemoveEmptyChunksPlugin
-      const dummyModule = new GriffelDummyModule();
+    // WHAT?
+    //  Forces all modules emitted by an extraction loader to be moved in a single chunk by SplitChunksPlugin config.
+    // WHY?
+    //  We need to sort CSS rules in the same order as it's done via style buckets. It's not possible in multiple
+    //  chunks.
+    if (compiler.options.optimization.splitChunks) {
+      compiler.options.optimization.splitChunks.cacheGroups ??= {};
+      compiler.options.optimization.splitChunks.cacheGroups['griffel'] = {
+        name: 'griffel',
+        test: isGriffelCSSModule,
+        chunks: 'all',
+        enforce: true,
+      };
+    }
 
-      if (this.attachToMainEntryPoint) {
-        compilation.hooks.afterChunks.tap(PLUGIN_NAME, () => {
-          attachGriffelChunkToMainEntryPoint(compilation, griffelChunk);
-        });
-      }
+    compiler.hooks.compilation.tap(PLUGIN_NAME, compilation => {
+      compilation.hooks.optimizeChunks.tap({ name: PLUGIN_NAME, stage: OPTIMIZE_CHUNKS_STAGE_ADVANCED }, () => {
+        // WHAT?
+        //   Performs module movements between chunks if SplitChunksPlugin is not enabled.
+        // WHY?
+        //   The same reason as for SplitChunksPlugin config.
+        if (!compiler.options.optimization.splitChunks) {
+          moveCSSModulesToGriffelChunk(compilation);
+        }
 
-      // Adds dummy module here to try to make sure that other module optimization steps won't conflict
-      compilation.hooks.afterOptimizeModules.tap(PLUGIN_NAME, () => {
-        compilation.modules.add(dummyModule);
-        compilation.chunkGraph.connectChunkAndModule(griffelChunk, dummyModule);
-      });
+        // WHAT?
+        //  Disconnects Griffel chunk from other chunks, so Griffel chunk cannot be loaded async. Also connects with
+        //  the main entrypoint in config.
+        // WHY?
+        //  This is scenario required by one of MS teams. Will be removed in the future.
+        if (this.attachToMainEntryPoint) {
+          const griffelChunk = compilation.namedChunks.get('griffel');
 
-      // Performs movements before SplitChunksPlugin
-      compilation.hooks.optimizeChunks.tap({ name: PLUGIN_NAME, stage: OPTIMIZE_CHUNKS_STAGE_BASIC }, () => {
-        moveCSSModulesToGriffelChunk(compilation, this.attachToMainEntryPoint);
-      });
+          if (typeof griffelChunk === 'undefined') {
+            return;
+          }
 
-      compilation.hooks.afterOptimizeChunks.tap(PLUGIN_NAME, () => {
-        // If SplitChunksPlugin has configured "cacheGroups", we need to move chunks again to ensure that needed
-        // modules are still attached "griffel" chunk
-        if (compiler.options.optimization.splitChunks && compiler.options.optimization.splitChunks.cacheGroups) {
-          moveCSSModulesToGriffelChunk(compilation, this.attachToMainEntryPoint);
+          griffelChunk.disconnectFromGroups();
+          attachGriffelChunkToMainEntryPoint(compilation, griffelChunk);
         }
-
-        // Remove dummy module once we are sure chunk optimization steps have finished. i.e. won't conflict with
-        // SplitChunksPlugin
-        compilation.modules.delete(dummyModule);
-        compilation.chunkGraph.disconnectChunkAndModule(griffelChunk, dummyModule);
       });
 
+      // WHAT?
+      //  Takes a CSS file from Griffel chunks and sorts CSS inside it.
       compilation.hooks.processAssets.tap(
         {
           name: PLUGIN_NAME,
           stage: Compilation.PROCESS_ASSETS_STAGE_PRE_PROCESS,
         },
         assets => {
+          const griffelChunk = compilation.namedChunks.get('griffel');
+
+          if (typeof griffelChunk === 'undefined') {
+            return;
+          }
+
           const cssAssetDetails = Object.entries(assets).find(([assetName]) => griffelChunk.files.has(assetName));
 
           if (typeof cssAssetDetails === 'undefined') {
diff --git a/packages/webpack-extraction-plugin/src/GriffelDummyModule.ts b/packages/webpack-extraction-plugin/src/GriffelDummyModule.ts
deleted file mode 100644
index 2782153..0000000
--- a/packages/webpack-extraction-plugin/src/GriffelDummyModule.ts
+++ /dev/null
@@ -1,28 +0,0 @@
-import { Module } from 'webpack';
-
-const TYPES = new Set(['unknown']);
-
-export class GriffelDummyModule extends Module {
-  readonly #identifier: string;
-
-  constructor() {
-    super('unknown', undefined);
-
-    this.#identifier = 'griffel-dummy-module';
-
-    this.buildInfo = {};
-    this.buildMeta = {};
-  }
-
-  override identifier() {
-    return this.#identifier;
-  }
-
-  override getSourceTypes() {
-    return TYPES;
-  }
-
-  override size() {
-    return 0;
-  }
-}


Test patch:
diff --git a/packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.test.ts b/packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.test.ts
index ba4d64c..f7f0262 100644
--- a/packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.test.ts
+++ b/packages/webpack-extraction-plugin/src/GriffelCSSExtractionPlugin.test.ts
@@ -242,6 +242,15 @@ describe('webpackLoader', () => {
   // Custom filenames in mini-css-extract-plugin
   testFixture('config-name', { cssFilename: '[name].[contenthash].css' });
 
+  // Config that disables SplitChunksPlugin
+  testFixture('config-no-split-chunks', {
+    webpackConfig: {
+      optimization: {
+        splitChunks: false,
+      },
+    },
+  });
+
   // Config that forces chunk splitting
   testFixture('config-split-chunks', {
     webpackConfig: {