From cb1038cb6d21bd6f51b70b3d4e52e03a46dc22b0 Mon Sep 17 00:00:00 2001
From: Shu Ding <g@shud.in>
Date: Tue, 23 Aug 2022 00:09:56 +0100
Subject: [PATCH] Refactor client CSS imports (#39758)

Removed the hack of client-side CSS injection via `chunk`. Instead collect them with the manifest plugin and SSR them as link tags.

Next step is to adjust HMR to not relying on mini-css-extract plugin and webpack.
---
 .../plugins/flight-client-entry-plugin.ts     | 19 +++----
 .../webpack/plugins/flight-manifest-plugin.ts | 51 +++++++------------
 packages/next/client/app-index.tsx            | 12 -----
 packages/next/server/app-render.tsx           |  4 +-
 .../app/app/css/css-client/client-foo.css     |  3 ++
 .../app/app/css/css-client/client-layout.css  |  3 ++
 .../app/app/css/css-client/client-page.css    |  6 +++
 .../e2e/app-dir/app/app/css/css-client/foo.js |  5 ++
 .../app/app/css/css-client/layout.client.js   | 12 +++++
 .../app/app/css/css-client/page.client.js     |  5 ++
 10 files changed, 66 insertions(+), 54 deletions(-)
 create mode 100644 test/e2e/app-dir/app/app/css/css-client/client-foo.css
 create mode 100644 test/e2e/app-dir/app/app/css/css-client/client-layout.css
 create mode 100644 test/e2e/app-dir/app/app/css/css-client/client-page.css
 create mode 100644 test/e2e/app-dir/app/app/css/css-client/foo.js
 create mode 100644 test/e2e/app-dir/app/app/css/css-client/layout.client.js
 create mode 100644 test/e2e/app-dir/app/app/css/css-client/page.client.js

diff --git a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts
index e67e005704b1a..358f8707bbacf 100644
--- a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts
+++ b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts
@@ -71,11 +71,6 @@ export class FlightClientEntryPlugin {
     // For each SC server compilation entry, we need to create its corresponding
     // client component entry.
     for (const [name, entry] of compilation.entries.entries()) {
-      // If the request is not for `app` directory entry skip it.
-      // if (!entry.request || !entry.request.startsWith('next-app-loader')) {
-      //   continue
-      // }
-
       // Check if the page entry is a server component or not.
       const entryDependency = entry.dependencies?.[0]
       // Ensure only next-app-loader entries are handled.
@@ -193,7 +188,10 @@ export class FlightClientEntryPlugin {
     const clientComponentImports: ClientComponentImports = []
     const serverCSSImports: CssImports = {}
 
-    const filterClientComponents = (dependencyToFilter: any): void => {
+    const filterClientComponents = (
+      dependencyToFilter: any,
+      inClientComponentBoundary: boolean
+    ): void => {
       const mod: webpack.NormalModule =
         compilation.moduleGraph.getResolvedModule(dependencyToFilter)
       if (!mod) return
@@ -236,7 +234,7 @@ export class FlightClientEntryPlugin {
       }
 
       // Check if request is for css file.
-      if (isClientComponent || isCSS) {
+      if ((!inClientComponentBoundary && isClientComponent) || isCSS) {
         clientComponentImports.push(modRequest)
         return
       }
@@ -244,12 +242,15 @@ export class FlightClientEntryPlugin {
       compilation.moduleGraph
         .getOutgoingConnections(mod)
         .forEach((connection: any) => {
-          filterClientComponents(connection.dependency)
+          filterClientComponents(
+            connection.dependency,
+            inClientComponentBoundary || isClientComponent
+          )
         })
     }
 
     // Traverse the module graph to find all client components.
-    filterClientComponents(dependency)
+    filterClientComponents(dependency, false)
 
     return [clientComponentImports, serverCSSImports]
   }
diff --git a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts
index 2dcc2997f0169..e3e721c06f8a7 100644
--- a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts
+++ b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts
@@ -114,6 +114,9 @@ export class FlightManifestPlugin {
     const dev = this.dev
 
     compilation.chunkGroups.forEach((chunkGroup) => {
+      const cssResourcesInChunkGroup: string[] = []
+      let entryFilepath: string = ''
+
       function recordModule(
         chunk: webpack.Chunk,
         id: ModuleId,
@@ -176,6 +179,11 @@ export class FlightManifestPlugin {
             }
             manifest.__ssr_module_mapping__ = moduleIdMapping
           }
+
+          if (chunkGroup.name) {
+            cssResourcesInChunkGroup.push(resource)
+          }
+
           return
         }
 
@@ -186,6 +194,10 @@ export class FlightManifestPlugin {
           return
         }
 
+        if (/\/(page|layout)\.client\.(ts|js)x?$/.test(resource)) {
+          entryFilepath = resource
+        }
+
         const exportsInfo = compilation.moduleGraph.getExportsInfo(mod)
         const cjsExports = [
           ...new Set([
@@ -218,37 +230,6 @@ export class FlightManifestPlugin {
           )
           .filter((name) => name !== null)
 
-        // Get all CSS files imported from the module's dependencies.
-        const visitedModule = new Set()
-        const cssChunks: Set<string> = new Set()
-
-        function collectClientImportedCss(module: any) {
-          if (!module) return
-
-          const modRequest = module.userRequest
-          if (visitedModule.has(modRequest)) return
-          visitedModule.add(modRequest)
-
-          if (/\.css$/.test(modRequest)) {
-            // collect relative imported css chunks
-            compilation.chunkGraph.getModuleChunks(module).forEach((c) => {
-              ;[...c.files]
-                .filter((file) => file.endsWith('.css'))
-                .forEach((file) => cssChunks.add(file))
-            })
-          }
-
-          const connections = Array.from(
-            compilation.moduleGraph.getOutgoingConnections(module)
-          )
-          connections.forEach((connection) => {
-            collectClientImportedCss(
-              compilation.moduleGraph.getResolvedModule(connection.dependency!)
-            )
-          })
-        }
-        collectClientImportedCss(mod)
-
         moduleExportedKeys.forEach((name) => {
           let requiredChunks: ManifestChunks = []
           if (!moduleExports[name]) {
@@ -273,7 +254,7 @@ export class FlightManifestPlugin {
             moduleExports[name] = {
               id,
               name,
-              chunks: requiredChunks.concat([...cssChunks]),
+              chunks: requiredChunks,
             }
           }
 
@@ -310,6 +291,12 @@ export class FlightManifestPlugin {
           }
         }
       })
+
+      const clientCSSManifest: any = manifest.__client_css_manifest__ || {}
+      if (entryFilepath) {
+        clientCSSManifest[entryFilepath] = cssResourcesInChunkGroup
+      }
+      manifest.__client_css_manifest__ = clientCSSManifest
     })
 
     const file = 'server/' + FLIGHT_MANIFEST
diff --git a/packages/next/client/app-index.tsx b/packages/next/client/app-index.tsx
index 9d0ec8accb6fa..4f148f9b74d49 100644
--- a/packages/next/client/app-index.tsx
+++ b/packages/next/client/app-index.tsx
@@ -32,18 +32,6 @@ self.__next_require__ = __webpack_require__
 // eslint-disable-next-line no-undef
 ;(self as any).__next_chunk_load__ = (chunk: string) => {
   if (!chunk) return Promise.resolve()
-  if (chunk.endsWith('.css')) {
-    // @ts-expect-error __webpack_public_path__ is inlined by webpack
-    const chunkPath = `${__webpack_public_path__ || ''}${chunk}`
-    const existingTag = document.querySelector(`link[href="${chunkPath}"]`)
-    if (!existingTag) {
-      const link = document.createElement('link')
-      link.rel = 'stylesheet'
-      link.href = chunkPath
-      document.head.appendChild(link)
-    }
-    return Promise.resolve()
-  }
   const [chunkId, chunkFileName] = chunk.split(':')
   chunkFilenameMap[chunkId] = `static/chunks/${chunkFileName}.js`
 
diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx
index 2c221e3e75e7d..012a0682d01ab 100644
--- a/packages/next/server/app-render.tsx
+++ b/packages/next/server/app-render.tsx
@@ -383,7 +383,9 @@ function getCssInlinedLinkTags(
   serverCSSManifest: FlightCSSManifest,
   filePath: string
 ): string[] {
-  const layoutOrPageCss = serverCSSManifest[filePath]
+  const layoutOrPageCss =
+    serverCSSManifest[filePath] ||
+    serverComponentManifest.__client_css_manifest__?.[filePath]
 
   if (!layoutOrPageCss) {
     return []
diff --git a/test/e2e/app-dir/app/app/css/css-client/client-foo.css b/test/e2e/app-dir/app/app/css/css-client/client-foo.css
new file mode 100644
index 0000000000000..94ff8d4b4895c
--- /dev/null
+++ b/test/e2e/app-dir/app/app/css/css-client/client-foo.css
@@ -0,0 +1,3 @@
+.foo {
+  color: blue;
+}
diff --git a/test/e2e/app-dir/app/app/css/css-client/client-layout.css b/test/e2e/app-dir/app/app/css/css-client/client-layout.css
new file mode 100644
index 0000000000000..5bc2906308a05
--- /dev/null
+++ b/test/e2e/app-dir/app/app/css/css-client/client-layout.css
@@ -0,0 +1,3 @@
+body {
+  background: cyan;
+}
diff --git a/test/e2e/app-dir/app/app/css/css-client/client-page.css b/test/e2e/app-dir/app/app/css/css-client/client-page.css
new file mode 100644
index 0000000000000..4b2edc961d9d2
--- /dev/null
+++ b/test/e2e/app-dir/app/app/css/css-client/client-page.css
@@ -0,0 +1,6 @@
+h1 {
+  color: red !important;
+}
+h1::after {
+  content: ' (from css-client!!)';
+}
diff --git a/test/e2e/app-dir/app/app/css/css-client/foo.js b/test/e2e/app-dir/app/app/css/css-client/foo.js
new file mode 100644
index 0000000000000..98d4f855cc4ed
--- /dev/null
+++ b/test/e2e/app-dir/app/app/css/css-client/foo.js
@@ -0,0 +1,5 @@
+import './client-foo.css'
+
+export default function Foo() {
+  return <b className="foo">foo</b>
+}
diff --git a/test/e2e/app-dir/app/app/css/css-client/layout.client.js b/test/e2e/app-dir/app/app/css/css-client/layout.client.js
new file mode 100644
index 0000000000000..2add562cce692
--- /dev/null
+++ b/test/e2e/app-dir/app/app/css/css-client/layout.client.js
@@ -0,0 +1,12 @@
+import './client-layout.css'
+
+import Foo from './foo'
+
+export default function ServerLayout({ children }) {
+  return (
+    <>
+      {children}
+      <Foo />
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/app/app/css/css-client/page.client.js b/test/e2e/app-dir/app/app/css/css-client/page.client.js
new file mode 100644
index 0000000000000..24df05926ff9e
--- /dev/null
+++ b/test/e2e/app-dir/app/app/css/css-client/page.client.js
@@ -0,0 +1,5 @@
+import './client-page.css'
+
+export default function Page() {
+  return <h1>Page!!!</h1>
+}