Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'),
Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/build/templates/edge-ssr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -106,6 +107,7 @@ const render = getRender({
buildManifest,
renderToHTML,
reactLoadableManifest,
dynamicCssManifest,
subresourceIntegrityManifest,
config: nextConfig,
buildId: process.env.__NEXT_BUILD_ID!,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -31,6 +34,7 @@ export function getRender({
Document,
buildManifest,
reactLoadableManifest,
dynamicCssManifest,
interceptionRouteRewrites,
renderToHTML,
clientReferenceManifest,
Expand All @@ -53,6 +57,7 @@ export function getRender({
Document: DocumentType
buildManifest: BuildManifest
reactLoadableManifest: ReactLoadableManifest
dynamicCssManifest?: DynamicCssManifest
subresourceIntegrityManifest?: Record<string, string>
interceptionRouteRewrites?: ManifestRewriteRoute[]
clientReferenceManifest?: ClientReferenceManifest
Expand All @@ -71,6 +76,7 @@ export function getRender({
dev,
buildManifest,
reactLoadableManifest,
dynamicCssManifest,
subresourceIntegrityManifest,
Document,
App: appMod?.default as AppType,
Expand Down
11 changes: 7 additions & 4 deletions packages/next/src/build/webpack/plugins/middleware-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -100,9 +101,7 @@ function getEntryFiles(
entryFiles: string[],
meta: EntryMetadata,
hasInstrumentationHook: boolean,
opts: {
sriEnabled: boolean
}
opts: Options
) {
const files: string[] = []
if (meta.edgeSSR) {
Expand All @@ -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`,
Expand All @@ -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) => {
Expand Down Expand Up @@ -810,6 +812,7 @@ export default class MiddlewarePlugin {
sriEnabled: this.sriEnabled,
rewrites: this.rewrites,
edgeEnvironments: this.edgeEnvironments,
dev: this.dev,
},
})
)
Expand Down
61 changes: 50 additions & 11 deletions packages/next/src/build/webpack/plugins/react-loadable-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -119,6 +131,10 @@ function buildManifest(
file.match(/^static\/(chunks|css)\//)
) {
files.add(file)

if (shouldCreateDynamicCssManifest && file.endsWith('.css')) {
dynamicCssManifestSet.add(file)
}
}
})
}
Expand All @@ -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

Expand All @@ -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
}
Expand All @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions packages/next/src/client/route-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 13 additions & 7 deletions packages/next/src/pages/_document.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ export class Head extends React.Component<HeadProps> {
assetPrefix,
assetQueryString,
dynamicImports,
dynamicCssManifest,
crossOrigin,
optimizeCss,
} = this.context
Expand All @@ -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(
Expand All @@ -469,7 +472,6 @@ export class Head extends React.Component<HeadProps> {
)
}

const isUnmanagedFile = unmanagedFiles.has(file)
cssLinkElements.push(
<link
key={file}
Expand All @@ -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
: ''
}
/>
)
})
Expand Down
16 changes: 16 additions & 0 deletions packages/next/src/server/load-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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`)
devjiwonchoi marked this conversation as resolved.
Show resolved Hide resolved
).catch(() => undefined),
hasClientManifest
? loadClientReferenceManifest(
join(
Expand Down Expand Up @@ -201,6 +216,7 @@ async function loadComponentsImpl<N = any>({
Component,
buildManifest,
reactLoadableManifest,
dynamicCssManifest,
pageConfig: ComponentMod.config || {},
ComponentMod,
getServerSideProps,
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/shared/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
5 changes: 5 additions & 0 deletions packages/next/src/shared/lib/html-context.shared-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
Expand Down
Loading
Loading