From 61199af29049ea44ca31c1bf6c503bce6cf36b90 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Mon, 19 Dec 2022 23:17:20 +0100 Subject: [PATCH 1/3] fix css resource duplicated issue --- packages/next/build/webpack-config.ts | 1 + .../plugins/flight-client-entry-plugin.ts | 10 ++-- .../webpack/plugins/flight-manifest-plugin.ts | 57 +++++++++++-------- .../next/client/components/layout-router.tsx | 2 +- packages/next/server/app-render.tsx | 38 ++++++++----- .../server/dev/on-demand-entry-handler.ts | 3 +- .../app/app/css/css-duplicate/a/page.js | 5 ++ .../app/app/css/css-duplicate/b/page.js | 5 ++ .../app-dir/app/app/css/css-duplicate/comp.js | 7 +++ .../app/app/css/css-duplicate/style.css | 3 + test/e2e/app-dir/index.test.ts | 23 ++++++++ 11 files changed, 110 insertions(+), 44 deletions(-) create mode 100644 test/e2e/app-dir/app/app/css/css-duplicate/a/page.js create mode 100644 test/e2e/app-dir/app/app/css/css-duplicate/b/page.js create mode 100644 test/e2e/app-dir/app/app/css/css-duplicate/comp.js create mode 100644 test/e2e/app-dir/app/app/css/css-duplicate/style.css diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index 18bc7e98ebf55d8..2e45309f67c849f 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -2106,6 +2106,7 @@ export default async function getBaseWebpackConfig( (isClient ? new FlightManifestPlugin({ dev, + appDir, }) : new FlightClientEntryPlugin({ appDir, 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 5dddca730f417b6..48b4177bcfacd9f 100644 --- a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts @@ -284,11 +284,11 @@ export class FlightClientEntryPlugin { } const entryCSSInfo: Record = - cssManifest.__entry_css__ || {} + cssManifest.__entry_css_mods__ || {} entryCSSInfo[entryName] = cssImportsForChunk[entryName] Object.assign(cssManifest, { - __entry_css__: entryCSSInfo, + __entry_css_mods__: entryCSSInfo, }) }) }) @@ -348,9 +348,9 @@ export class FlightClientEntryPlugin { const manifest = JSON.stringify({ ...serverCSSManifest, ...edgeServerCSSManifest, - __entry_css__: { - ...serverCSSManifest.__entry_css__, - ...edgeServerCSSManifest.__entry_css__, + __entry_css_mods__: { + ...serverCSSManifest.__entry_css_mods__, + ...edgeServerCSSManifest.__entry_css_mods__, }, }) assets[FLIGHT_SERVER_CSS_MANIFEST + '.json'] = new sources.RawSource( diff --git a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts index 7f0cd647638841b..47a7ef95819eb2d 100644 --- a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts @@ -26,6 +26,7 @@ import { traverseModules } from '../utils' interface Options { dev: boolean + appDir: string } /** @@ -65,6 +66,9 @@ export type FlightManifest = { __edge_ssr_module_mapping__: { [moduleId: string]: ManifestNode } + __entry_css_files__: { + [entryPathWithoutExtension: string]: string[] + } } & { [modulePath: string]: ManifestNode } @@ -72,7 +76,7 @@ export type FlightManifest = { export type FlightCSSManifest = { [modulePath: string]: string[] } & { - __entry_css__?: { + __entry_css_mods__?: { [entry: string]: string[] } } @@ -86,9 +90,11 @@ export const ASYNC_CLIENT_MODULES = new Set() export class FlightManifestPlugin { dev: Options['dev'] = false + appDir: Options['appDir'] constructor(options: Options) { this.dev = options.dev + this.appDir = options.appDir } apply(compiler: webpack.Compiler) { @@ -127,6 +133,7 @@ export class FlightManifestPlugin { const manifest: FlightManifest = { __ssr_module_mapping__: {}, __edge_ssr_module_mapping__: {}, + __entry_css_files__: {}, } const dev = this.dev @@ -145,13 +152,10 @@ export class FlightManifestPlugin { traverseModules(compilation, (mod) => collectClientRequest(mod)) compilation.chunkGroups.forEach((chunkGroup) => { - const cssResourcesInChunkGroup = new Set() - let entryFilepath: string = '' - function recordModule( - chunk: webpack.Chunk, id: ModuleId, - mod: webpack.NormalModule + mod: webpack.NormalModule, + chunkCSS: string[] ) { const isCSSModule = regexCSS.test(mod.resource) || @@ -191,13 +195,12 @@ export class FlightManifestPlugin { ssrNamedModuleId = `./${ssrNamedModuleId.replace(/\\/g, '/')}` if (isCSSModule) { - const chunks = [...chunk.files].filter((f) => f.endsWith('.css')) if (!manifest[resource]) { manifest[resource] = { default: { id, name: 'default', - chunks, + chunks: chunkCSS, }, } } else { @@ -205,14 +208,10 @@ export class FlightManifestPlugin { // e.g. extracted by mini-css-extract-plugin. In that case we need to // merge the chunks. manifest[resource].default.chunks = [ - ...new Set([...manifest[resource].default.chunks, ...chunks]), + ...new Set([...manifest[resource].default.chunks, ...chunkCSS]), ] } - if (chunkGroup.name) { - cssResourcesInChunkGroup.add(resource) - } - return } @@ -222,10 +221,6 @@ export class FlightManifestPlugin { return } - if (/[\\/](page|layout)\.(ts|js)x?$/.test(resource)) { - entryFilepath = resource - } - const exportsInfo = compilation.moduleGraph.getExportsInfo(mod) const isAsyncModule = ASYNC_CLIENT_MODULES.has(mod.resource) @@ -326,27 +321,43 @@ export class FlightManifestPlugin { chunk // TODO: Update type so that it doesn't have to be cast. ) as Iterable + + const chunkCSS = [...chunk.files].filter( + (f) => !f.startsWith('static/css/pages/') && f.endsWith('.css') + ) + for (const mod of chunkModules) { const modId: string = compilation.chunkGraph.getModuleId(mod) + '' - recordModule(chunk, modId, mod) + recordModule(modId, mod, chunkCSS) // If this is a concatenation, register each child to the parent ID. // TODO: remove any const anyModule = mod as any if (anyModule.modules) { anyModule.modules.forEach((concatenatedMod: any) => { - recordModule(chunk, modId, concatenatedMod) + recordModule(modId, concatenatedMod, chunkCSS) }) } } }) - const clientCSSManifest: any = manifest.__client_css_manifest__ || {} - if (entryFilepath) { - clientCSSManifest[entryFilepath] = Array.from(cssResourcesInChunkGroup) + let entryName = chunkGroup.name + + // Can be created from a child entry. + if (!entryName) { + entryName = chunkGroup.getParents()[0]?.options.name + } + + const entryCSSFiles: any = manifest.__entry_css_files__ || {} + if (entryName?.startsWith('app/')) { + const key = this.appDir + entryName.slice(3) + entryCSSFiles[key] = chunkGroup + .getFiles() + .filter((f) => f.endsWith('.css')) + .concat(entryCSSFiles[key] || []) } - manifest.__client_css_manifest__ = clientCSSManifest + manifest.__entry_css_files__ = entryCSSFiles }) const file = 'server/' + FLIGHT_MANIFEST diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index aa482626e8780c9..69a7fb215eb4906 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -6,10 +6,10 @@ import type { import type { FlightRouterState, FlightSegmentPath, + ChildProp, } from '../../server/app-render' import type { ErrorComponent } from './error-boundary' import type { FocusAndScrollRef } from './reducer' -import type { ChildProp } from '../../server/app-render' import React, { useContext, useEffect, use } from 'react' import ReactDOM from 'react-dom' diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 5a1bd0f5c45dd59..e3d45ac6bf98238 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -684,24 +684,34 @@ function getCssInlinedLinkTags( filePath: string, serverCSSForEntries: string[] ): string[] { - const layoutOrPageCss = - serverCSSManifest[filePath] || - serverComponentManifest.__client_css_manifest__?.[filePath] + const layoutOrPageCssModules = serverCSSManifest[filePath] - if (!layoutOrPageCss) { + const filePathWithoutExt = filePath.replace(/\.[^.]+$/, '') + const cssFilesForEntry = new Set( + serverComponentManifest.__entry_css_files__?.[filePathWithoutExt] || [] + ) + + // console.log( + // filePath, + // cssFilesForEntry, + // serverComponentManifest.__entry_css_files__ + // ) + + if (!layoutOrPageCssModules || !cssFilesForEntry.size) { return [] } - const chunks = new Set() - for (const css of layoutOrPageCss) { + 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(css) || !/\.module\.css/.test(css)) { - const mod = serverComponentManifest[css] - if (mod) { - for (const chunk of mod.default.chunks) { - chunks.add(chunk) + 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) + } } } } @@ -718,10 +728,10 @@ function getServerCSSForEntries( for (const entry of entries) { const entryName = entry.replace(/\.[^.]+$/, '') if ( - serverCSSManifest.__entry_css__ && - serverCSSManifest.__entry_css__[entryName] + serverCSSManifest.__entry_css_mods__ && + serverCSSManifest.__entry_css_mods__[entryName] ) { - css.push(...serverCSSManifest.__entry_css__[entryName]) + css.push(...serverCSSManifest.__entry_css_mods__[entryName]) } } return css diff --git a/packages/next/server/dev/on-demand-entry-handler.ts b/packages/next/server/dev/on-demand-entry-handler.ts index 051e4d19abe336a..5299a4da7923a71 100644 --- a/packages/next/server/dev/on-demand-entry-handler.ts +++ b/packages/next/server/dev/on-demand-entry-handler.ts @@ -2,6 +2,8 @@ import type ws from 'ws' import origDebug from 'next/dist/compiled/debug' import type { webpack } from 'next/dist/compiled/webpack/webpack' import type { NextConfigComplete } from '../config-shared' +import type { DynamicParamTypesShort, FlightRouterState } from '../app-render' + import { EventEmitter } from 'events' import { findPageFile } from '../lib/find-page-file' import { runDependingOnPageType } from '../../build/entries' @@ -15,7 +17,6 @@ import getRouteFromEntrypoint from '../get-route-from-entrypoint' import { getPageStaticInfo } from '../../build/analysis/get-page-static-info' import { isMiddlewareFile, isMiddlewareFilename } from '../../build/utils' import { PageNotFoundError } from '../../shared/lib/utils' -import { DynamicParamTypesShort, FlightRouterState } from '../app-render' import { CompilerNameValues, COMPILER_INDEXES, diff --git a/test/e2e/app-dir/app/app/css/css-duplicate/a/page.js b/test/e2e/app-dir/app/app/css/css-duplicate/a/page.js new file mode 100644 index 000000000000000..6828e6f3c7e10a3 --- /dev/null +++ b/test/e2e/app-dir/app/app/css/css-duplicate/a/page.js @@ -0,0 +1,5 @@ +import Comp from '../comp' + +export default function Page() { + return +} diff --git a/test/e2e/app-dir/app/app/css/css-duplicate/b/page.js b/test/e2e/app-dir/app/app/css/css-duplicate/b/page.js new file mode 100644 index 000000000000000..6828e6f3c7e10a3 --- /dev/null +++ b/test/e2e/app-dir/app/app/css/css-duplicate/b/page.js @@ -0,0 +1,5 @@ +import Comp from '../comp' + +export default function Page() { + return +} diff --git a/test/e2e/app-dir/app/app/css/css-duplicate/comp.js b/test/e2e/app-dir/app/app/css/css-duplicate/comp.js new file mode 100644 index 000000000000000..e0ae75039e247f0 --- /dev/null +++ b/test/e2e/app-dir/app/app/css/css-duplicate/comp.js @@ -0,0 +1,7 @@ +'use client' + +import './style.css' + +export default function Comp() { + return

Hello

+} diff --git a/test/e2e/app-dir/app/app/css/css-duplicate/style.css b/test/e2e/app-dir/app/app/css/css-duplicate/style.css new file mode 100644 index 000000000000000..c6dd2c88fabe351 --- /dev/null +++ b/test/e2e/app-dir/app/app/css/css-duplicate/style.css @@ -0,0 +1,3 @@ +.red { + color: red; +} diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 8a5cf0ab95cc6ec..65990da508e4e22 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1467,6 +1467,29 @@ createNextDescribe( ).toBe('50px') }) }) + + if (isDev) { + describe('multiple entries', () => { + 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') + + const browser = await next.browser('/css/css-duplicate/a') + expect( + await browser.eval( + `[...document.styleSheets].some(({ href }) => href.endsWith('/a/page.css'))` + ) + ).toBe(true) + + // Should not load the chunk from /b + expect( + await browser.eval( + `[...document.styleSheets].some(({ href }) => href.endsWith('/b/page.css'))` + ) + ).toBe(false) + }) + }) + } }) if (isDev) { From fd0e98e88c579cadd1e2c79274e2dc30cd9d5ce3 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 20 Dec 2022 17:36:04 +0100 Subject: [PATCH 2/3] fix chunkGroup tracking --- .../webpack/plugins/flight-manifest-plugin.ts | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts index 78c588e0c7f77fe..9ac29f51266af6d 100644 --- a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts @@ -342,21 +342,28 @@ export class FlightManifestPlugin { } }) - let entryName = chunkGroup.name + const entryCSSFiles: any = manifest.__entry_css_files__ || {} - // Can be created from a child entry. - if (!entryName) { - entryName = chunkGroup.getParents()[0]?.options.name + const addCSSFilesToEntry = ( + files: string[], + entryName: string | undefined | null + ) => { + if (entryName?.startsWith('app/')) { + const key = this.appDir + entryName.slice(3) + entryCSSFiles[key] = files.concat(entryCSSFiles[key] || []) + } } - const entryCSSFiles: any = manifest.__entry_css_files__ || {} - if (entryName?.startsWith('app/')) { - const key = this.appDir + entryName.slice(3) - entryCSSFiles[key] = chunkGroup - .getFiles() - .filter((f) => f.endsWith('.css')) - .concat(entryCSSFiles[key] || []) + const cssFiles = chunkGroup.getFiles().filter((f) => f.endsWith('.css')) + + if (cssFiles.length) { + // Add to chunk entry and parent chunk groups too. + addCSSFilesToEntry(cssFiles, chunkGroup.name) + chunkGroup.getParents().forEach((parent) => { + addCSSFilesToEntry(cssFiles, parent.options.name) + }) } + manifest.__entry_css_files__ = entryCSSFiles }) From cb7f7807ff610a39d2ff651dc10e4682931398a9 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 20 Dec 2022 17:44:39 +0100 Subject: [PATCH 3/3] remove comments --- packages/next/server/app-render.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index e3d45ac6bf98238..3a12c1ac7551323 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -691,12 +691,6 @@ function getCssInlinedLinkTags( serverComponentManifest.__entry_css_files__?.[filePathWithoutExt] || [] ) - // console.log( - // filePath, - // cssFilesForEntry, - // serverComponentManifest.__entry_css_files__ - // ) - if (!layoutOrPageCssModules || !cssFilesForEntry.size) { return [] }