From 8bbe8b6b461171d47aca46e8c2eaebac6f058066 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Mon, 16 Jan 2023 22:42:30 +0100 Subject: [PATCH 1/5] fix css imports being tracked multiple times --- .../webpack/loaders/get-module-build-info.ts | 7 ++ .../build/webpack/loaders/next-app-loader.ts | 74 ++++++++++++++----- .../plugins/flight-client-entry-plugin.ts | 53 +++++++++++-- .../app/css/css-duplicate-2/client/page.js | 7 ++ .../app/app/css/css-duplicate-2/layout.js | 5 ++ .../app/css/css-duplicate-2/server/page.js | 5 ++ .../app/css/css-duplicate-2/style.module.css | 3 + test/e2e/app-dir/app/index.test.ts | 21 ++++++ 8 files changed, 149 insertions(+), 26 deletions(-) create mode 100644 test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js create mode 100644 test/e2e/app-dir/app/app/css/css-duplicate-2/layout.js create mode 100644 test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js create mode 100644 test/e2e/app-dir/app/app/css/css-duplicate-2/style.module.css diff --git a/packages/next/src/build/webpack/loaders/get-module-build-info.ts b/packages/next/src/build/webpack/loaders/get-module-build-info.ts index bd3dd8a32b31a..60376b8d885de 100644 --- a/packages/next/src/build/webpack/loaders/get-module-build-info.ts +++ b/packages/next/src/build/webpack/loaders/get-module-build-info.ts @@ -21,9 +21,16 @@ export function getModuleBuildInfo(webpackModule: webpack.Module) { importLocByPath?: Map rootDir?: string rsc?: RSCMeta + treeInfo?: TreeNode } } +// This attaches the app tree structure to the build into +export interface TreeNode { + resource: string + children: TreeNode[] +} + export interface RSCMeta { type?: RSCModuleType requests?: string[] // client requests in flight client entry diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts index 2fade9d19f356..b440747fbeb97 100644 --- a/packages/next/src/build/webpack/loaders/next-app-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-app-loader.ts @@ -2,7 +2,7 @@ import type webpack from 'webpack' import chalk from 'next/dist/compiled/chalk' import type { ValueOf } from '../../../shared/lib/constants' import { NODE_RESOLVE_OPTIONS } from '../../webpack-config' -import { getModuleBuildInfo } from './get-module-build-info' +import { getModuleBuildInfo, TreeNode } from './get-module-build-info' import { sep } from 'path' import { verifyRootLayout } from '../../../lib/verifyRootLayout' import * as Log from '../../../build/output/log' @@ -49,7 +49,10 @@ async function createTreeCodeFromPath({ async function createSubtreePropsFromSegmentPath( segments: string[] - ): Promise { + ): Promise<{ + tree: TreeNode + treeCode: string + }> { const segmentPath = segments.join('/') // Existing tree are the children of the current segment @@ -63,12 +66,18 @@ async function createTreeCodeFromPath({ parallelSegments.push(...resolveParallelSegments(segmentPath)) } + const tree: TreeNode = { + resource: '', + children: [], + } for (const [parallelKey, parallelSegment] of parallelSegments) { if (parallelSegment === PAGE_SEGMENT) { const matchedPagePath = `${appDirPrefix}${segmentPath}/page` const resolvedPagePath = await resolve(matchedPagePath) if (resolvedPagePath) pages.push(resolvedPagePath) + tree.resource = resolvedPagePath || '' + // Use '' for segment as it's the page. There can't be a segment called '' so this is the safest way to add it. props[parallelKey] = `['', {}, { page: [() => import(/* webpackMode: "eager" */ ${JSON.stringify( @@ -77,11 +86,17 @@ async function createTreeCodeFromPath({ continue } + const parallelTree: TreeNode = { + resource: '', + children: [], + } + tree.children.push(parallelTree) + const parallelSegmentPath = segmentPath + '/' + parallelSegment - const subtree = await createSubtreePropsFromSegmentPath([ - ...segments, - parallelSegment, - ]) + const { tree: subtree, treeCode: subtreeCode } = + await createSubtreePropsFromSegmentPath([...segments, parallelSegment]) + + parallelTree.children.push(subtree) // `page` is not included here as it's added above. const filePaths = await Promise.all( @@ -93,10 +108,14 @@ async function createTreeCodeFromPath({ }) ) + const layoutPath = filePaths.find( + ([type, path]) => type === 'layout' && !!path + )?.[1] + if (layoutPath) { + parallelTree.resource = layoutPath + } if (!rootLayout) { - rootLayout = filePaths.find( - ([type, path]) => type === 'layout' && !!path - )?.[1] + rootLayout = layoutPath } if (!globalError) { @@ -107,7 +126,7 @@ async function createTreeCodeFromPath({ props[parallelKey] = `[ '${parallelSegment}', - ${subtree}, + ${subtreeCode}, { ${filePaths .filter(([, filePath]) => filePath !== undefined) @@ -115,6 +134,14 @@ async function createTreeCodeFromPath({ if (filePath === undefined) { return '' } + + if (file !== 'layout') { + parallelTree.children.push({ + resource: filePath, + children: [], + }) + } + return `'${file}': [() => import(/* webpackMode: "eager" */ ${JSON.stringify( filePath )}), ${JSON.stringify(filePath)}],` @@ -124,15 +151,24 @@ async function createTreeCodeFromPath({ ]` } - return `{ - ${Object.entries(props) - .map(([key, value]) => `${key}: ${value}`) - .join(',\n')} - }` + return { + tree, + treeCode: `{ + ${Object.entries(props) + .map(([key, value]) => `${key}: ${value}`) + .join(',\n')} + }`, + } } - const tree = await createSubtreePropsFromSegmentPath([]) - return [`const tree = ${tree}.children;`, pages, rootLayout, globalError] + const { treeCode, tree } = await createSubtreePropsFromSegmentPath([]) + return { + tree, + treeCode: `const tree = ${treeCode}.children;`, + pages, + rootLayout, + globalError, + } } function createAbsolutePath(appDir: string, pathToTurnAbsolute: string) { @@ -220,7 +256,7 @@ const nextAppLoader: webpack.LoaderDefinitionFunction<{ } } - const [treeCode, pages, rootLayout, globalError] = + const { tree, treeCode, pages, rootLayout, globalError } = await createTreeCodeFromPath({ pagePath, resolve: resolver, @@ -251,6 +287,8 @@ const nextAppLoader: webpack.LoaderDefinitionFunction<{ } } + buildInfo.treeInfo = tree + const result = ` export ${treeCode} export const pages = ${JSON.stringify(pages)} diff --git a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts index be2293d82ae94..deed62c4fe201 100644 --- a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts @@ -25,6 +25,7 @@ import { ASYNC_CLIENT_MODULES } from './flight-manifest-plugin' import { isClientComponentModule, regexCSS } from '../loaders/utils' import { traverseModules } from '../utils' import { normalizePathSep } from '../../../shared/lib/page-path/normalize-path-sep' +import { TreeNode } from '../loaders/get-module-build-info' interface Options { dev: boolean @@ -236,7 +237,7 @@ export class FlightClientEntryPlugin { // After optimizing all the modules, we collect the CSS that are still used // by the certain chunk. compilation.hooks.afterOptimizeModules.tap(PLUGIN_NAME, () => { - const cssImportsForChunk: Record = {} + const cssImportsForChunk: Record> = {} if (this.isEdgeServer) { edgeServerCSSManifest = {} } else { @@ -252,7 +253,7 @@ export class FlightClientEntryPlugin { const modId = resource if (modId) { if (regexCSS.test(modId)) { - cssImportsForChunk[entryName].push(modId) + cssImportsForChunk[entryName].add(modId) } } } @@ -266,7 +267,7 @@ export class FlightClientEntryPlugin { const entryName = path.join(this.appDir, '..', chunk.name) if (!cssImportsForChunk[entryName]) { - cssImportsForChunk[entryName] = [] + cssImportsForChunk[entryName] = new Set() } const chunkModules = compilation.chunkGraph.getChunkModulesIterable( @@ -285,7 +286,7 @@ export class FlightClientEntryPlugin { const entryCSSInfo: Record = cssManifest.__entry_css_mods__ || {} - entryCSSInfo[entryName] = cssImportsForChunk[entryName] + entryCSSInfo[entryName] = [...cssImportsForChunk[entryName]] Object.assign(cssManifest, { __entry_css_mods__: entryCSSInfo, @@ -318,12 +319,18 @@ export class FlightClientEntryPlugin { } }) + const tracked = new Set() for (const connection of compilation.moduleGraph.getOutgoingConnections( entryModule )) { const entryDependency = connection.dependency const entryRequest = connection.dependency.request + // It is possible that the same entry is added multiple times in the + // connection graph. We can just skip these to speed up the process. + if (tracked.has(entryRequest)) continue + tracked.add(entryRequest) + const [, cssImports] = this.collectClientComponentsAndCSSForDependency({ entryRequest, @@ -334,6 +341,36 @@ export class FlightClientEntryPlugin { Object.assign(cssManifest, cssImports) } + + // Finally, we need to clean up CSS imports a bit. If a parent or + // current layout already imports a CSS file, we can remove it from + // the current entry's CSS imports. + const { treeInfo } = entryModule.buildInfo + if (treeInfo) { + function filterIncludedCSS( + tree: TreeNode, + parentCSSImports: Set + ) { + const used = new Set(parentCSSImports) + + // Remove CSS imports that are already imported by the parent + const collected = cssManifest[tree.resource] + if (collected?.length) { + cssManifest[tree.resource] = collected.filter((cssImport) => { + if (!parentCSSImports.has(cssImport)) { + used.add(cssImport) + return true + } + }) + } + + // Recursively check children + tree.children.forEach((child) => { + filterIncludedCSS(child, used) + }) + } + filterIncludedCSS(treeInfo, new Set()) + } }) }) @@ -403,7 +440,7 @@ export class FlightClientEntryPlugin { */ const visitedBySegment: { [segment: string]: Set } = {} const clientComponentImports: ClientComponentImports = [] - const serverCSSImports: CssImports = {} + const CSSImports: CssImports = {} const filterClientComponents = ( dependencyToFilter: any, @@ -451,8 +488,8 @@ export class FlightClientEntryPlugin { } } - serverCSSImports[entryRequest] = serverCSSImports[entryRequest] || [] - serverCSSImports[entryRequest].push(modRequest) + CSSImports[entryRequest] = CSSImports[entryRequest] || [] + CSSImports[entryRequest].push(modRequest) } // Check if request is for css file. @@ -483,7 +520,7 @@ export class FlightClientEntryPlugin { // Traverse the module graph to find all client components. filterClientComponents(dependency, false) - return [clientComponentImports, serverCSSImports] + return [clientComponentImports, CSSImports] } injectClientEntryAndSSRModules({ diff --git a/test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js b/test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js new file mode 100644 index 0000000000000..862a12c09be02 --- /dev/null +++ b/test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js @@ -0,0 +1,7 @@ +'use client' + +import styles from '../style.module.css' + +export default function Page({}) { + return
Hello
+} diff --git a/test/e2e/app-dir/app/app/css/css-duplicate-2/layout.js b/test/e2e/app-dir/app/app/css/css-duplicate-2/layout.js new file mode 100644 index 0000000000000..bb5affda2c4e4 --- /dev/null +++ b/test/e2e/app-dir/app/app/css/css-duplicate-2/layout.js @@ -0,0 +1,5 @@ +import styles from './style.module.css' + +export default function Layout({ children }) { + return
{children}
+} diff --git a/test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js b/test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js new file mode 100644 index 0000000000000..d932435b585a2 --- /dev/null +++ b/test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js @@ -0,0 +1,5 @@ +import styles from '../style.module.css' + +export default function Page({}) { + return
Hello
+} diff --git a/test/e2e/app-dir/app/app/css/css-duplicate-2/style.module.css b/test/e2e/app-dir/app/app/css/css-duplicate-2/style.module.css new file mode 100644 index 0000000000000..36d9096c8210e --- /dev/null +++ b/test/e2e/app-dir/app/app/css/css-duplicate-2/style.module.css @@ -0,0 +1,3 @@ +.foo::before { + content: '_randomized_string_for_testing_'; +} diff --git a/test/e2e/app-dir/app/index.test.ts b/test/e2e/app-dir/app/index.test.ts index 0885a8c4cdbb5..f3e02c78ea0a7 100644 --- a/test/e2e/app-dir/app/index.test.ts +++ b/test/e2e/app-dir/app/index.test.ts @@ -1466,6 +1466,27 @@ createNextDescribe( if (isDev) { describe('multiple entries', () => { + it('should only inject the same style once if used by different layers', async () => { + const browser = await next.browser('/css/css-duplicate-2/client') + expect( + await browser.eval( + `[...document.styleSheets].filter(({ cssRules }) => + [...cssRules].some(({ cssText }) => (cssText||'').includes('_randomized_string_for_testing_')) + ).length` + ) + ).toBe(1) + }) + + it('should only include the same style once in the flight data', async () => { + const initialHtml = await next.render('/css/css-duplicate-2/server') + + // Even if it's deduped by Float, it should still only be included once in the payload. + // There are two matches, one for the rendered and one for the flight data. + expect( + initialHtml.match(/duplicate-2_style_module_css\.css/g).length + ).toBe(2) + }) + it('should only load chunks for the css module that is used by the specific entrypoint', async () => { // Visit /b first await next.render('/css/css-duplicate/b') From c2b88b5a0732f5cae20b3a65af2eebe1a50a99ab Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Mon, 16 Jan 2023 22:57:32 +0100 Subject: [PATCH 2/5] lint errors --- .../src/build/webpack/plugins/flight-client-entry-plugin.ts | 1 + test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js | 2 +- test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts index deed62c4fe201..7a2dc69231700 100644 --- a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts @@ -361,6 +361,7 @@ export class FlightClientEntryPlugin { used.add(cssImport) return true } + return false }) } diff --git a/test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js b/test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js index 862a12c09be02..59a1fdae2f1bc 100644 --- a/test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js +++ b/test/e2e/app-dir/app/app/css/css-duplicate-2/client/page.js @@ -2,6 +2,6 @@ import styles from '../style.module.css' -export default function Page({}) { +export default function Page() { return
Hello
} diff --git a/test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js b/test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js index d932435b585a2..fa11f6dbd9511 100644 --- a/test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js +++ b/test/e2e/app-dir/app/app/css/css-duplicate-2/server/page.js @@ -1,5 +1,5 @@ import styles from '../style.module.css' -export default function Page({}) { +export default function Page() { return
Hello
} From 7eb0f5f80ca30f9de2b82dbadc91bd3cf5e9a8f1 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Mon, 16 Jan 2023 22:58:19 +0100 Subject: [PATCH 3/5] fix typo --- .../next/src/build/webpack/loaders/get-module-build-info.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/build/webpack/loaders/get-module-build-info.ts b/packages/next/src/build/webpack/loaders/get-module-build-info.ts index 60376b8d885de..bc32fa4d2409e 100644 --- a/packages/next/src/build/webpack/loaders/get-module-build-info.ts +++ b/packages/next/src/build/webpack/loaders/get-module-build-info.ts @@ -25,7 +25,7 @@ export function getModuleBuildInfo(webpackModule: webpack.Module) { } } -// This attaches the app tree structure to the build into +// This attaches the app tree structure to the build info export interface TreeNode { resource: string children: TreeNode[] From 00b7bc99d2416adde533b818ce9aaa752065a57c Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 17 Jan 2023 17:18:50 +0100 Subject: [PATCH 4/5] refactor code --- .../webpack/loaders/get-module-build-info.ts | 7 -- .../build/webpack/loaders/next-app-loader.ts | 40 ++--------- .../plugins/flight-client-entry-plugin.ts | 32 --------- packages/next/src/server/app-render.tsx | 72 ++++++++++++++++--- 4 files changed, 68 insertions(+), 83 deletions(-) diff --git a/packages/next/src/build/webpack/loaders/get-module-build-info.ts b/packages/next/src/build/webpack/loaders/get-module-build-info.ts index bc32fa4d2409e..bd3dd8a32b31a 100644 --- a/packages/next/src/build/webpack/loaders/get-module-build-info.ts +++ b/packages/next/src/build/webpack/loaders/get-module-build-info.ts @@ -21,16 +21,9 @@ export function getModuleBuildInfo(webpackModule: webpack.Module) { importLocByPath?: Map rootDir?: string rsc?: RSCMeta - treeInfo?: TreeNode } } -// This attaches the app tree structure to the build info -export interface TreeNode { - resource: string - children: TreeNode[] -} - export interface RSCMeta { type?: RSCModuleType requests?: string[] // client requests in flight client entry diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts index b440747fbeb97..b6ee93ff9ec23 100644 --- a/packages/next/src/build/webpack/loaders/next-app-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-app-loader.ts @@ -2,7 +2,7 @@ import type webpack from 'webpack' import chalk from 'next/dist/compiled/chalk' import type { ValueOf } from '../../../shared/lib/constants' import { NODE_RESOLVE_OPTIONS } from '../../webpack-config' -import { getModuleBuildInfo, TreeNode } from './get-module-build-info' +import { getModuleBuildInfo } from './get-module-build-info' import { sep } from 'path' import { verifyRootLayout } from '../../../lib/verifyRootLayout' import * as Log from '../../../build/output/log' @@ -50,7 +50,6 @@ async function createTreeCodeFromPath({ async function createSubtreePropsFromSegmentPath( segments: string[] ): Promise<{ - tree: TreeNode treeCode: string }> { const segmentPath = segments.join('/') @@ -66,18 +65,12 @@ async function createTreeCodeFromPath({ parallelSegments.push(...resolveParallelSegments(segmentPath)) } - const tree: TreeNode = { - resource: '', - children: [], - } for (const [parallelKey, parallelSegment] of parallelSegments) { if (parallelSegment === PAGE_SEGMENT) { const matchedPagePath = `${appDirPrefix}${segmentPath}/page` const resolvedPagePath = await resolve(matchedPagePath) if (resolvedPagePath) pages.push(resolvedPagePath) - tree.resource = resolvedPagePath || '' - // Use '' for segment as it's the page. There can't be a segment called '' so this is the safest way to add it. props[parallelKey] = `['', {}, { page: [() => import(/* webpackMode: "eager" */ ${JSON.stringify( @@ -86,17 +79,10 @@ async function createTreeCodeFromPath({ continue } - const parallelTree: TreeNode = { - resource: '', - children: [], - } - tree.children.push(parallelTree) - const parallelSegmentPath = segmentPath + '/' + parallelSegment - const { tree: subtree, treeCode: subtreeCode } = - await createSubtreePropsFromSegmentPath([...segments, parallelSegment]) - - parallelTree.children.push(subtree) + const { treeCode: subtreeCode } = await createSubtreePropsFromSegmentPath( + [...segments, parallelSegment] + ) // `page` is not included here as it's added above. const filePaths = await Promise.all( @@ -111,9 +97,6 @@ async function createTreeCodeFromPath({ const layoutPath = filePaths.find( ([type, path]) => type === 'layout' && !!path )?.[1] - if (layoutPath) { - parallelTree.resource = layoutPath - } if (!rootLayout) { rootLayout = layoutPath } @@ -135,13 +118,6 @@ async function createTreeCodeFromPath({ return '' } - if (file !== 'layout') { - parallelTree.children.push({ - resource: filePath, - children: [], - }) - } - return `'${file}': [() => import(/* webpackMode: "eager" */ ${JSON.stringify( filePath )}), ${JSON.stringify(filePath)}],` @@ -152,7 +128,6 @@ async function createTreeCodeFromPath({ } return { - tree, treeCode: `{ ${Object.entries(props) .map(([key, value]) => `${key}: ${value}`) @@ -161,9 +136,8 @@ async function createTreeCodeFromPath({ } } - const { treeCode, tree } = await createSubtreePropsFromSegmentPath([]) + const { treeCode } = await createSubtreePropsFromSegmentPath([]) return { - tree, treeCode: `const tree = ${treeCode}.children;`, pages, rootLayout, @@ -256,7 +230,7 @@ const nextAppLoader: webpack.LoaderDefinitionFunction<{ } } - const { tree, treeCode, pages, rootLayout, globalError } = + const { treeCode, pages, rootLayout, globalError } = await createTreeCodeFromPath({ pagePath, resolve: resolver, @@ -287,8 +261,6 @@ const nextAppLoader: webpack.LoaderDefinitionFunction<{ } } - buildInfo.treeInfo = tree - const result = ` export ${treeCode} export const pages = ${JSON.stringify(pages)} diff --git a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts index 7a2dc69231700..f40345dcf7754 100644 --- a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts @@ -25,7 +25,6 @@ import { ASYNC_CLIENT_MODULES } from './flight-manifest-plugin' import { isClientComponentModule, regexCSS } from '../loaders/utils' import { traverseModules } from '../utils' import { normalizePathSep } from '../../../shared/lib/page-path/normalize-path-sep' -import { TreeNode } from '../loaders/get-module-build-info' interface Options { dev: boolean @@ -341,37 +340,6 @@ export class FlightClientEntryPlugin { Object.assign(cssManifest, cssImports) } - - // Finally, we need to clean up CSS imports a bit. If a parent or - // current layout already imports a CSS file, we can remove it from - // the current entry's CSS imports. - const { treeInfo } = entryModule.buildInfo - if (treeInfo) { - function filterIncludedCSS( - tree: TreeNode, - parentCSSImports: Set - ) { - const used = new Set(parentCSSImports) - - // Remove CSS imports that are already imported by the parent - const collected = cssManifest[tree.resource] - if (collected?.length) { - cssManifest[tree.resource] = collected.filter((cssImport) => { - if (!parentCSSImports.has(cssImport)) { - used.add(cssImport) - return true - } - return false - }) - } - - // Recursively check children - tree.children.forEach((child) => { - filterIncludedCSS(child, used) - }) - } - filterIncludedCSS(treeInfo, new Set()) - } }) }) diff --git a/packages/next/src/server/app-render.tsx b/packages/next/src/server/app-render.tsx index 3b7f113f90ea9..a756ba8329a3e 100644 --- a/packages/next/src/server/app-render.tsx +++ b/packages/next/src/server/app-render.tsx @@ -704,7 +704,9 @@ function getCssInlinedLinkTags( serverComponentManifest: FlightManifest, serverCSSManifest: FlightCSSManifest, filePath: string, - serverCSSForEntries: string[] + serverCSSForEntries: string[], + injectedCSS: Set, + collectNewCSSImports?: boolean ): string[] { const layoutOrPageCssModules = serverCSSManifest[filePath] @@ -721,12 +723,26 @@ function getCssInlinedLinkTags( for (const mod of layoutOrPageCssModules) { // We only include the CSS if it's a global CSS, or it is used by this // entrypoint. - if (serverCSSForEntries.includes(mod) || !/\.module\.css/.test(mod)) { - const modData = serverComponentManifest[mod] - if (modData) { - for (const chunk of modData.default.chunks) { - if (cssFilesForEntry.has(chunk)) { - chunks.add(chunk) + if ( + serverCSSForEntries.includes(mod) || + !/\.module\.(css|sass)$/.test(mod) + ) { + // If the CSS is already injected by a parent layer, we don't need + // to inject it again. + if (!injectedCSS.has(mod)) { + const modData = serverComponentManifest[mod] + if (modData) { + for (const chunk of modData.default.chunks) { + // If the current entry in the final tree-shaked bundle has that CSS + // chunk, it means that it's actually used. We should include it. + if (cssFilesForEntry.has(chunk)) { + chunks.add(chunk) + // This might be a new layout, and to make it more efficient and + // not introducing another loop, we mutate the set directly. + if (collectNewCSSImports) { + injectedCSS.add(mod) + } + } } } } @@ -1098,16 +1114,19 @@ export async function renderToHTMLOrFlight( filePath, getComponent, shouldPreload, + injectedCSS, }: { filePath: string getComponent: () => any shouldPreload?: boolean + injectedCSS: Set }): Promise => { const cssHrefs = getCssInlinedLinkTags( serverComponentManifest, serverCSSManifest, filePath, - serverCSSForEntries + serverCSSForEntries, + injectedCSS ) const styles = cssHrefs @@ -1144,20 +1163,26 @@ export async function renderToHTMLOrFlight( parentParams, firstItem, rootLayoutIncluded, + injectedCSS, }: { createSegmentPath: CreateSegmentPath loaderTree: LoaderTree parentParams: { [key: string]: any } rootLayoutIncluded?: boolean firstItem?: boolean + injectedCSS: Set }): Promise<{ Component: React.ComponentType }> => { const layoutOrPagePath = layout?.[1] || page?.[1] + + const injectedCSSWithCurrentLayout = new Set(injectedCSS) const stylesheets: string[] = layoutOrPagePath ? getCssInlinedLinkTags( serverComponentManifest, serverCSSManifest!, layoutOrPagePath, - serverCSSForEntries + serverCSSForEntries, + injectedCSSWithCurrentLayout, + true ) : [] @@ -1176,6 +1201,7 @@ export async function renderToHTMLOrFlight( filePath: template[1], getComponent: template[0], shouldPreload: true, + injectedCSS: injectedCSSWithCurrentLayout, }) : [React.Fragment] @@ -1183,6 +1209,7 @@ export async function renderToHTMLOrFlight( ? await createComponentAndStyles({ filePath: error[1], getComponent: error[0], + injectedCSS: injectedCSSWithCurrentLayout, }) : [] @@ -1190,6 +1217,7 @@ export async function renderToHTMLOrFlight( ? await createComponentAndStyles({ filePath: loading[1], getComponent: loading[0], + injectedCSS: injectedCSSWithCurrentLayout, }) : [] @@ -1215,6 +1243,7 @@ export async function renderToHTMLOrFlight( ? await createComponentAndStyles({ filePath: notFound[1], getComponent: notFound[0], + injectedCSS: injectedCSSWithCurrentLayout, }) : rootLayoutAtThisLevel ? [DefaultNotFound] @@ -1354,6 +1383,7 @@ export async function renderToHTMLOrFlight( loaderTree: parallelRoutes[parallelRouteKey], parentParams: currentParams, rootLayoutIncluded: rootLayoutIncludedAtThisLevelOrAbove, + injectedCSS: injectedCSSWithCurrentLayout, }) const childProp: ChildProp = { @@ -1500,6 +1530,7 @@ export async function renderToHTMLOrFlight( flightRouterState, parentRendered, rscPayloadHead, + injectedCSS, }: { createSegmentPath: CreateSegmentPath loaderTreeToFilter: LoaderTree @@ -1508,8 +1539,9 @@ export async function renderToHTMLOrFlight( flightRouterState?: FlightRouterState parentRendered?: boolean rscPayloadHead: React.ReactNode + injectedCSS: Set }): Promise => { - const [segment, parallelRoutes] = loaderTreeToFilter + const [segment, parallelRoutes, { layout }] = loaderTreeToFilter const parallelRoutesKeys = Object.keys(parallelRoutes) // Because this function walks to a deeper point in the tree to start rendering we have to track the dynamic parameters up to the point where rendering starts @@ -1559,6 +1591,7 @@ export async function renderToHTMLOrFlight( loaderTree: loaderTreeToFilter, parentParams: currentParams, firstItem: isFirst, + injectedCSS, } ) return @@ -1569,6 +1602,22 @@ export async function renderToHTMLOrFlight( ] } + // If we are not rendering on this level we need to check if the current + // segment has a layout. If so, we need to track all the used CSS to make + // the result consistent. + const layoutPath = layout?.[1] + const injectedCSSWithCurrentLayout = new Set(injectedCSS) + if (layoutPath) { + getCssInlinedLinkTags( + serverComponentManifest, + serverCSSManifest!, + layoutPath, + serverCSSForEntries, + injectedCSSWithCurrentLayout, + true + ) + } + // Walk through all parallel routes. for (const parallelRouteKey of parallelRoutesKeys) { const parallelRoute = parallelRoutes[parallelRouteKey] @@ -1588,6 +1637,7 @@ export async function renderToHTMLOrFlight( parentRendered: parentRendered || renderComponentsOnThisLevel, isFirst: false, rscPayloadHead, + injectedCSS: injectedCSSWithCurrentLayout, }) if (typeof path[path.length - 1] !== 'string') { @@ -1610,6 +1660,7 @@ export async function renderToHTMLOrFlight( flightRouterState: providedFlightRouterState, isFirst: true, rscPayloadHead, + injectedCSS: new Set(), }) ).slice(1), ] @@ -1688,6 +1739,7 @@ export async function renderToHTMLOrFlight( loaderTree: loaderTree, parentParams: {}, firstItem: true, + injectedCSS: new Set(), }) const initialTree = createFlightRouterStateFromLoaderTree(loaderTree) From 9092f12f6998b13cbb22e6cf4fc762e5a781d9dd Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 17 Jan 2023 18:02:23 +0100 Subject: [PATCH 5/5] scss --- packages/next/src/server/app-render.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/server/app-render.tsx b/packages/next/src/server/app-render.tsx index 139c507cf5a88..80981fac3d2d5 100644 --- a/packages/next/src/server/app-render.tsx +++ b/packages/next/src/server/app-render.tsx @@ -725,7 +725,7 @@ function getCssInlinedLinkTags( // entrypoint. if ( serverCSSForEntries.includes(mod) || - !/\.module\.(css|sass)$/.test(mod) + !/\.module\.(css|sass|scss)$/.test(mod) ) { // If the CSS is already injected by a parent layer, we don't need // to inject it again.