fix(pages): dynamic css missing style after client navigation (#72959)

> [!NOTE] > This issue occurs only on: > - Pages Router > - Production > - Webpack Build ### Why? When client-side navigating from Page 1 to Page 2, where Page 1 **statically imports** CSS, whereas Page 2 **dynamically imports** CSS, the expected style is missing at Page 2. The root cause of the issue is the `mini-css-extract-plugin` (which handles Production CSS) skipped injecting the stylesheet since the `link` tag with the target `href` already existed. This is fine, but the expected stylesheet is missing as Next.js removes "server-rendered" stylesheets after the navigation. ![mermaid-diagram-2024-11-06-202137](https://github.com/user-attachments/assets/4ddb3454-29d3-4ef1-b782-50e4f863263f) ### How? Create a `dynamic-css-manifest` with the list of dynamic CSS files along with the `react-loadables-manifest`. Then, pass it to the internal `_document`'s `<Head>` component. During rendering the head, if the href of the target CSS is included in the dynamic CSS files list, do not add the `data-n-p` (attribute for server-rendered CSS) attribute. This is possible because we do not "unload" the dynamic stylesheets during the client navigation. Therefore, the result will be the same with removing the current stylesheet and then dynamically loading the same stylesheet. ### Testing Plan - Covers runtime `nodejs` and `edge` - `next/dynamic` - React `lazy` - dynamic `import()` Fixes #33286 Fixes #47655 Fixes #68328 Closes NDX-145 --------- Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com> Co-authored-by: JJ Kasper <jj@jjsweb.site>
vercel · Nov 29, 2024 · 3d82475 · 3d82475
1 parent 97939e9
commit 3d82475
Show file tree

Hide file tree

Showing 31 changed files with 402 additions and 23 deletions.
diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts
@@ -79,6 +79,7 @@ import {
   FUNCTIONS_CONFIG_MANIFEST,
   UNDERSCORE_NOT_FOUND_ROUTE_ENTRY,
   UNDERSCORE_NOT_FOUND_ROUTE,
+  DYNAMIC_CSS_MANIFEST,
 } from '../shared/lib/constants'
 import {
   getSortedRoutes,
@@ -2613,6 +2614,12 @@ export default async function build(
                     ),
                   ]
                 : []),
+              ...(pagesDir && !turboNextBuild
+                ? [
+                    DYNAMIC_CSS_MANIFEST + '.json',
+                    path.join(SERVER_DIRECTORY, DYNAMIC_CSS_MANIFEST + '.js'),
+                  ]
+                : []),
               REACT_LOADABLE_MANIFEST,
               BUILD_ID_FILE,
               path.join(SERVER_DIRECTORY, NEXT_FONT_MANIFEST + '.js'),

diff --git a/packages/next/src/build/templates/edge-ssr.ts b/packages/next/src/build/templates/edge-ssr.ts
@@ -89,6 +89,7 @@ const maybeJSONParse = (str?: string) => (str ? JSON.parse(str) : undefined)
 
 const buildManifest: BuildManifest = self.__BUILD_MANIFEST as any
 const reactLoadableManifest = maybeJSONParse(self.__REACT_LOADABLE_MANIFEST)
+const dynamicCssManifest = maybeJSONParse(self.__DYNAMIC_CSS_MANIFEST)
 const subresourceIntegrityManifest = sriEnabled
   ? maybeJSONParse(self.__SUBRESOURCE_INTEGRITY_MANIFEST)
   : undefined
@@ -106,6 +107,7 @@ const render = getRender({
   buildManifest,
   renderToHTML,
   reactLoadableManifest,
+  dynamicCssManifest,
   subresourceIntegrityManifest,
   config: nextConfig,
   buildId: process.env.__NEXT_BUILD_ID!,

diff --git a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts
@@ -2,7 +2,10 @@ import type { NextConfigComplete } from '../../../../server/config-shared'
 
 import type { DocumentType, AppType } from '../../../../shared/lib/utils'
 import type { BuildManifest } from '../../../../server/get-page-files'
-import type { ReactLoadableManifest } from '../../../../server/load-components'
+import type {
+  DynamicCssManifest,
+  ReactLoadableManifest,
+} from '../../../../server/load-components'
 import type { ClientReferenceManifest } from '../../plugins/flight-manifest-plugin'
 import type { NextFontManifest } from '../../plugins/next-font-manifest-plugin'
 import type { NextFetchEvent } from '../../../../server/web/spec-extension/fetch-event'
@@ -31,6 +34,7 @@ export function getRender({
   Document,
   buildManifest,
   reactLoadableManifest,
+  dynamicCssManifest,
   interceptionRouteRewrites,
   renderToHTML,
   clientReferenceManifest,
@@ -53,6 +57,7 @@ export function getRender({
   Document: DocumentType
   buildManifest: BuildManifest
   reactLoadableManifest: ReactLoadableManifest
+  dynamicCssManifest?: DynamicCssManifest
   subresourceIntegrityManifest?: Record<string, string>
   interceptionRouteRewrites?: ManifestRewriteRoute[]
   clientReferenceManifest?: ClientReferenceManifest
@@ -71,6 +76,7 @@ export function getRender({
     dev,
     buildManifest,
     reactLoadableManifest,
+    dynamicCssManifest,
     subresourceIntegrityManifest,
     Document,
     App: appMod?.default as AppType,

diff --git a/packages/next/src/build/webpack/plugins/middleware-plugin.ts b/packages/next/src/build/webpack/plugins/middleware-plugin.ts
@@ -21,6 +21,7 @@ import {
   NEXT_FONT_MANIFEST,
   SERVER_REFERENCE_MANIFEST,
   INTERCEPTION_ROUTE_REWRITE_MANIFEST,
+  DYNAMIC_CSS_MANIFEST,
 } from '../../../shared/lib/constants'
 import type { MiddlewareConfig } from '../../analysis/get-page-static-info'
 import type { Telemetry } from '../../../telemetry/storage'
@@ -100,9 +101,7 @@ function getEntryFiles(
   entryFiles: string[],
   meta: EntryMetadata,
   hasInstrumentationHook: boolean,
-  opts: {
-    sriEnabled: boolean
-  }
+  opts: Options
 ) {
   const files: string[] = []
   if (meta.edgeSSR) {
@@ -124,6 +123,9 @@ function getEntryFiles(
           )
       )
     }
+    if (!opts.dev && !meta.edgeSSR.isAppDir) {
+      files.push(`server/${DYNAMIC_CSS_MANIFEST}.js`)
+    }
 
     files.push(
       `server/${MIDDLEWARE_BUILD_MANIFEST}.js`,
@@ -149,7 +151,7 @@ function getEntryFiles(
 function getCreateAssets(params: {
   compilation: webpack.Compilation
   metadataByEntry: Map<string, EntryMetadata>
-  opts: Omit<Options, 'dev'>
+  opts: Options
 }) {
   const { compilation, metadataByEntry, opts } = params
   return (assets: any) => {
@@ -810,6 +812,7 @@ export default class MiddlewarePlugin {
             sriEnabled: this.sriEnabled,
             rewrites: this.rewrites,
             edgeEnvironments: this.edgeEnvironments,
+            dev: this.dev,
           },
         })
       )

diff --git a/packages/next/src/build/webpack/plugins/react-loadable-plugin.ts b/packages/next/src/build/webpack/plugins/react-loadable-plugin.ts
@@ -21,9 +21,13 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWAR
 // Implementation of this PR: https://github.com/jamiebuilds/react-loadable/pull/132
 // Modified to strip out unneeded results for Next's specific use case
 
-import { webpack, sources } from 'next/dist/compiled/webpack/webpack'
-
+import type {
+  DynamicCssManifest,
+  ReactLoadableManifest,
+} from '../../../server/load-components'
 import path from 'path'
+import { webpack, sources } from 'next/dist/compiled/webpack/webpack'
+import { DYNAMIC_CSS_MANIFEST } from '../../../shared/lib/constants'
 
 function getModuleId(compilation: any, module: any): string | number {
   return compilation.chunkGraph.getModuleId(module)
@@ -54,12 +58,20 @@ function buildManifest(
   _compiler: webpack.Compiler,
   compilation: webpack.Compilation,
   projectSrcDir: string | undefined,
-  dev: boolean
-) {
+  dev: boolean,
+  shouldCreateDynamicCssManifest: boolean
+): {
+  reactLoadableManifest: ReactLoadableManifest
+  dynamicCssManifest: DynamicCssManifest
+} {
   if (!projectSrcDir) {
-    return {}
+    return {
+      reactLoadableManifest: {},
+      dynamicCssManifest: [],
+    }
   }
-  let manifest: { [k: string]: { id: string | number; files: string[] } } = {}
+  const dynamicCssManifestSet = new Set<string>()
+  let manifest: ReactLoadableManifest = {}
 
   // This is allowed:
   // import("./module"); <- ImportDependency
@@ -119,6 +131,10 @@ function buildManifest(
                 file.match(/^static\/(chunks|css)\//)
               ) {
                 files.add(file)
+
+                if (shouldCreateDynamicCssManifest && file.endsWith('.css')) {
+                  dynamicCssManifestSet.add(file)
+                }
               }
             })
           }
@@ -143,12 +159,16 @@ function buildManifest(
     // eslint-disable-next-line no-sequences
     .reduce((a, c) => ((a[c] = manifest[c]), a), {} as any)
 
-  return manifest
+  return {
+    reactLoadableManifest: manifest,
+    dynamicCssManifest: Array.from(dynamicCssManifestSet),
+  }
 }
 
 export class ReactLoadablePlugin {
   private filename: string
   private pagesOrAppDir: string | undefined
+  private isPagesDir: boolean
   private runtimeAsset?: string
   private dev: boolean
 
@@ -161,6 +181,7 @@ export class ReactLoadablePlugin {
   }) {
     this.filename = opts.filename
     this.pagesOrAppDir = opts.pagesDir || opts.appDir
+    this.isPagesDir = Boolean(opts.pagesDir)
     this.runtimeAsset = opts.runtimeAsset
     this.dev = opts.dev
   }
@@ -169,23 +190,41 @@ export class ReactLoadablePlugin {
     const projectSrcDir = this.pagesOrAppDir
       ? path.dirname(this.pagesOrAppDir)
       : undefined
-    const manifest = buildManifest(
+    const shouldCreateDynamicCssManifest = !this.dev && this.isPagesDir
+    const { reactLoadableManifest, dynamicCssManifest } = buildManifest(
       compiler,
       compilation,
       projectSrcDir,
-      this.dev
+      this.dev,
+      shouldCreateDynamicCssManifest
     )
 
     assets[this.filename] = new sources.RawSource(
-      JSON.stringify(manifest, null, 2)
+      JSON.stringify(reactLoadableManifest, null, 2)
     )
     if (this.runtimeAsset) {
       assets[this.runtimeAsset] = new sources.RawSource(
         `self.__REACT_LOADABLE_MANIFEST=${JSON.stringify(
-          JSON.stringify(manifest)
+          JSON.stringify(reactLoadableManifest)
         )}`
       )
     }
+
+    // This manifest prevents removing server rendered <link> tags after client
+    // navigation. This is only needed under Pages dir && Production && Webpack.
+    // x-ref: https://github.com/vercel/next.js/pull/72959
+    if (shouldCreateDynamicCssManifest) {
+      assets[`${DYNAMIC_CSS_MANIFEST}.json`] = new sources.RawSource(
+        JSON.stringify(dynamicCssManifest, null, 2)
+      )
+      // This is for edge runtime.
+      assets[`server/${DYNAMIC_CSS_MANIFEST}.js`] = new sources.RawSource(
+        `self.__DYNAMIC_CSS_MANIFEST=${JSON.stringify(
+          JSON.stringify(dynamicCssManifest)
+        )}`
+      )
+    }
+
     return assets
   }
 

diff --git a/packages/next/src/client/route-loader.ts b/packages/next/src/client/route-loader.ts
@@ -19,6 +19,7 @@ declare global {
     __MIDDLEWARE_MATCHERS?: MiddlewareMatcher[]
     __MIDDLEWARE_MANIFEST_CB?: Function
     __REACT_LOADABLE_MANIFEST?: any
+    __DYNAMIC_CSS_MANIFEST?: any
     __RSC_MANIFEST?: any
     __RSC_SERVER_MANIFEST?: any
     __NEXT_FONT_MANIFEST?: any

diff --git a/packages/next/src/pages/_document.tsx b/packages/next/src/pages/_document.tsx
@@ -429,6 +429,7 @@ export class Head extends React.Component<HeadProps> {
       assetPrefix,
       assetQueryString,
       dynamicImports,
+      dynamicCssManifest,
       crossOrigin,
       optimizeCss,
     } = this.context
@@ -438,21 +439,23 @@ export class Head extends React.Component<HeadProps> {
     // Unmanaged files are CSS files that will be handled directly by the
     // webpack runtime (`mini-css-extract-plugin`).
     let unmanagedFiles: Set<string> = new Set([])
-    let dynamicCssFiles = Array.from(
+    let localDynamicCssFiles = Array.from(
       new Set(dynamicImports.filter((file) => file.endsWith('.css')))
     )
-    if (dynamicCssFiles.length) {
+    if (localDynamicCssFiles.length) {
       const existing = new Set(cssFiles)
-      dynamicCssFiles = dynamicCssFiles.filter(
+      localDynamicCssFiles = localDynamicCssFiles.filter(
         (f) => !(existing.has(f) || sharedFiles.has(f))
       )
-      unmanagedFiles = new Set(dynamicCssFiles)
-      cssFiles.push(...dynamicCssFiles)
+      unmanagedFiles = new Set(localDynamicCssFiles)
+      cssFiles.push(...localDynamicCssFiles)
     }
 
     let cssLinkElements: JSX.Element[] = []
     cssFiles.forEach((file) => {
       const isSharedFile = sharedFiles.has(file)
+      const isUnmanagedFile = unmanagedFiles.has(file)
+      const isFileInDynamicCssManifest = dynamicCssManifest.has(file)
 
       if (!optimizeCss) {
         cssLinkElements.push(
@@ -469,7 +472,6 @@ export class Head extends React.Component<HeadProps> {
         )
       }
 
-      const isUnmanagedFile = unmanagedFiles.has(file)
       cssLinkElements.push(
         <link
           key={file}
@@ -480,7 +482,11 @@ export class Head extends React.Component<HeadProps> {
           )}${assetQueryString}`}
           crossOrigin={this.props.crossOrigin || crossOrigin}
           data-n-g={isUnmanagedFile ? undefined : isSharedFile ? '' : undefined}
-          data-n-p={isUnmanagedFile ? undefined : isSharedFile ? undefined : ''}
+          data-n-p={
+            isSharedFile || isUnmanagedFile || isFileInDynamicCssManifest
+              ? undefined
+              : ''
+          }
         />
       )
     })

diff --git a/packages/next/src/server/load-components.ts b/packages/next/src/server/load-components.ts
@@ -19,6 +19,7 @@ import {
   REACT_LOADABLE_MANIFEST,
   CLIENT_REFERENCE_MANIFEST,
   SERVER_REFERENCE_MANIFEST,
+  DYNAMIC_CSS_MANIFEST,
 } from '../shared/lib/constants'
 import { join } from 'path'
 import { requirePage } from './require'
@@ -38,6 +39,12 @@ export type ManifestItem = {
 }
 
 export type ReactLoadableManifest = { [moduleId: string]: ManifestItem }
+/**
+ * This manifest prevents removing server rendered <link> tags after client
+ * navigation. This is only needed under `Pages dir && Production && Webpack`.
+ * @see https://github.com/vercel/next.js/pull/72959
+ */
+export type DynamicCssManifest = string[]
 
 /**
  * A manifest entry type for the react-loadable-manifest.json.
@@ -56,6 +63,7 @@ export type LoadComponentsReturnType<NextModule = any> = {
   buildManifest: DeepReadonly<BuildManifest>
   subresourceIntegrityManifest?: DeepReadonly<Record<string, string>>
   reactLoadableManifest: DeepReadonly<ReactLoadableManifest>
+  dynamicCssManifest?: DeepReadonly<DynamicCssManifest>
   clientReferenceManifest?: DeepReadonly<ClientReferenceManifest>
   serverActionsManifest?: any
   Document: DocumentType
@@ -147,13 +155,20 @@ async function loadComponentsImpl<N = any>({
   const [
     buildManifest,
     reactLoadableManifest,
+    dynamicCssManifest,
     clientReferenceManifest,
     serverActionsManifest,
   ] = await Promise.all([
     loadManifestWithRetries<BuildManifest>(join(distDir, BUILD_MANIFEST)),
     loadManifestWithRetries<ReactLoadableManifest>(
       join(distDir, REACT_LOADABLE_MANIFEST)
     ),
+    // This manifest will only exist in Pages dir && Production && Webpack.
+    isAppPath || process.env.TURBOPACK
+      ? undefined
+      : loadManifestWithRetries<DynamicCssManifest>(
+          join(distDir, `${DYNAMIC_CSS_MANIFEST}.json`)
+        ).catch(() => undefined),
     hasClientManifest
       ? loadClientReferenceManifest(
           join(
@@ -201,6 +216,7 @@ async function loadComponentsImpl<N = any>({
     Component,
     buildManifest,
     reactLoadableManifest,
+    dynamicCssManifest,
     pageConfig: ComponentMod.config || {},
     ComponentMod,
     getServerSideProps,

diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx
@@ -1472,6 +1472,7 @@ export async function renderToHTMLImpl(
     isDevelopment: !!dev,
     hybridAmp,
     dynamicImports: Array.from(dynamicImports),
+    dynamicCssManifest: new Set(renderOpts.dynamicCssManifest || []),
     assetPrefix,
     // Only enabled in production as development mode has features relying on HMR (style injection for example)
     unstable_runtimeJS:

diff --git a/packages/next/src/shared/lib/constants.ts b/packages/next/src/shared/lib/constants.ts
@@ -75,6 +75,8 @@ export const MIDDLEWARE_REACT_LOADABLE_MANIFEST =
 // server/interception-route-rewrite-manifest.js
 export const INTERCEPTION_ROUTE_REWRITE_MANIFEST =
   'interception-route-rewrite-manifest'
+// server/dynamic-css-manifest.js
+export const DYNAMIC_CSS_MANIFEST = 'dynamic-css-manifest'
 
 // static/runtime/main.js
 export const CLIENT_STATIC_FILES_RUNTIME_MAIN = `main`

diff --git a/packages/next/src/shared/lib/html-context.shared-runtime.ts b/packages/next/src/shared/lib/html-context.shared-runtime.ts
@@ -22,6 +22,11 @@ export type HtmlProps = {
   hybridAmp: boolean
   isDevelopment: boolean
   dynamicImports: string[]
+  /**
+   * This manifest is only needed for Pages dir, Production, Webpack
+   * @see https://github.com/vercel/next.js/pull/72959
+   */
+  dynamicCssManifest: Set<string>
   assetPrefix?: string
   canonicalBase: string
   headTags: any[]