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 0d96cda58cb25..b33689891a0aa 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 @@ -92,6 +92,66 @@ const pluginState = getProxiedPluginState({ injectedClientEntries: {} as Record, }) +function deduplicateCSSImportsForEntry(mergedCSSimports: CssImports) { + // If multiple entry module connections are having the same CSS import, + // we only need to have one module to keep track of that CSS import. + // It is based on the fact that if a page or a layout is rendered in the + // given entry, all its parent layouts are always rendered too. + // This can avoid duplicate CSS imports in the generated CSS manifest, + // for example, if a page and its parent layout are both using the same + // CSS import, we only need to have the layout to keep track of that CSS + // import. + // To achieve this, we need to first collect all the CSS imports from + // every connection, and deduplicate them in the order of layers from + // top to bottom. The implementation can be generally described as: + // - Sort by number of `/` in the request path (the more `/`, the deeper) + // - When in the same depth, sort by the filename (template < layout < page and others) + + // Sort the connections as described above. + const sortedCSSImports = Object.entries(mergedCSSimports).sort((a, b) => { + const [aPath] = a + const [bPath] = b + + const aDepth = aPath.split('/').length + const bDepth = bPath.split('/').length + + if (aDepth !== bDepth) { + return aDepth - bDepth + } + + const aName = path.parse(aPath).name + const bName = path.parse(bPath).name + + const indexA = ['template', 'layout'].indexOf(aName) + const indexB = ['template', 'layout'].indexOf(bName) + + if (indexA === -1) return 1 + if (indexB === -1) return -1 + return indexA - indexB + }) + + const dedupedCSSImports: CssImports = {} + const trackedCSSImports = new Set() + for (const [entryName, cssImports] of sortedCSSImports) { + for (const cssImport of cssImports) { + if (trackedCSSImports.has(cssImport)) continue + + // Only track CSS imports that are in files that can inherit CSS. + const filename = path.parse(entryName).name + if (['template', 'layout'].includes(filename)) { + trackedCSSImports.add(cssImport) + } + + if (!dedupedCSSImports[entryName]) { + dedupedCSSImports[entryName] = [] + } + dedupedCSSImports[entryName].push(cssImport) + } + } + + return dedupedCSSImports +} + export class ClientReferenceEntryPlugin { dev: boolean appDir: string @@ -196,6 +256,8 @@ export class ClientReferenceEntryPlugin { ClientComponentImports[0] >() const actionEntryImports = new Map() + const clientEntriesToInject = [] + const mergedCSSimports: CssImports = {} for (const connection of compilation.moduleGraph.getOutgoingConnections( entryModule @@ -204,7 +266,7 @@ export class ClientReferenceEntryPlugin { const entryDependency = connection.dependency const entryRequest = connection.dependency.request - const { clientImports, actionImports } = + const { clientComponentImports, actionImports, cssImports } = this.collectComponentInfoFromDependencies({ entryRequest, compilation, @@ -219,7 +281,7 @@ export class ClientReferenceEntryPlugin { // Next.js internals are put into a separate entry. if (!isAbsoluteRequest) { - clientImports.forEach((value) => + clientComponentImports.forEach((value) => internalClientComponentEntryImports.add(value) ) continue @@ -234,14 +296,30 @@ export class ClientReferenceEntryPlugin { relativeRequest.replace(/\.[^.\\/]+$/, '').replace(/^src[\\/]/, '') ) + Object.assign(mergedCSSimports, cssImports) + clientEntriesToInject.push({ + compiler, + compilation, + entryName: name, + clientComponentImports, + cssImports, + bundlePath, + absolutePagePath: entryRequest, + }) + } + + // Make sure CSS imports are deduplicated before injecting the client entry + // and SSR modules. + const dedupedCSSImports = deduplicateCSSImportsForEntry(mergedCSSimports) + for (const clientEntryToInject of clientEntriesToInject) { addClientEntryAndSSRModulesList.push( this.injectClientEntryAndSSRModules({ - compiler, - compilation, - entryName: name, - clientImports, - bundlePath, - absolutePagePath: entryRequest, + ...clientEntryToInject, + clientImports: [ + ...clientEntryToInject.clientComponentImports, + ...(dedupedCSSImports[clientEntryToInject.absolutePagePath] || + []), + ], }) ) } @@ -447,6 +525,8 @@ export class ClientReferenceEntryPlugin { forEachEntryModule(compilation, ({ name, entryModule }) => { const clientEntryDependencyMap = collectClientEntryDependencyMap(name) const tracked = new Set() + const mergedCSSimports: CssImports = {} + for (const connection of compilation.moduleGraph.getOutgoingConnections( entryModule )) { @@ -465,9 +545,14 @@ export class ClientReferenceEntryPlugin { clientEntryDependencyMap, }) - if (!cssManifest.cssImports) cssManifest.cssImports = {} - Object.assign(cssManifest.cssImports, cssImports) + Object.assign(mergedCSSimports, cssImports) } + + if (!cssManifest.cssImports) cssManifest.cssImports = {} + Object.assign( + cssManifest.cssImports, + deduplicateCSSImportsForEntry(mergedCSSimports) + ) }) }) @@ -534,8 +619,8 @@ export class ClientReferenceEntryPlugin { dependency: any /* Dependency */ clientEntryDependencyMap?: Record }): { - clientImports: ClientComponentImports cssImports: CssImports + clientComponentImports: ClientComponentImports actionImports: [string, string[]][] clientActionImports: [string, string[]][] } { @@ -605,13 +690,12 @@ export class ClientReferenceEntryPlugin { CSSImports.add(modRequest) } - // Check if request is for css file. - if ((!inClientComponentBoundary && isClientComponent) || isCSS) { + if (!inClientComponentBoundary && isClientComponent) { clientComponentImports.push(modRequest) // Here we are entering a client boundary, and we need to collect dependencies // in the client graph too. - if (isClientComponent && clientEntryDependencyMap) { + if (clientEntryDependencyMap) { if (clientEntryDependencyMap[modRequest]) { filterClientComponents(clientEntryDependencyMap[modRequest], true) } @@ -637,7 +721,7 @@ export class ClientReferenceEntryPlugin { } return { - clientImports: clientComponentImports, + clientComponentImports, cssImports: CSSImports.size ? { [entryRequest]: Array.from(CSSImports), diff --git a/test/e2e/app-dir/app-css/app/css/css-conflict-layers/blue-button.js b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/blue-button.js new file mode 100644 index 0000000000000..e4db41d232873 --- /dev/null +++ b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/blue-button.js @@ -0,0 +1,6 @@ +import { Button } from './button' +import styles from './blue-button.module.css' + +export function BlueButton() { + return +} diff --git a/test/e2e/app-dir/app-css/app/css/css-conflict-layers/blue-button.module.css b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/blue-button.module.css new file mode 100644 index 0000000000000..0cbe4551ec7d6 --- /dev/null +++ b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/blue-button.module.css @@ -0,0 +1,4 @@ +.blue-button { + color: white; + background: blue; +} diff --git a/test/e2e/app-dir/app-css/app/css/css-conflict-layers/button.js b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/button.js new file mode 100644 index 0000000000000..04c5980ca6487 --- /dev/null +++ b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/button.js @@ -0,0 +1,5 @@ +import styles from './button.module.css' + +export function Button({ className = '' }) { + return
Button
+} diff --git a/test/e2e/app-dir/app-css/app/css/css-conflict-layers/button.module.css b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/button.module.css new file mode 100644 index 0000000000000..9afc6b501074f --- /dev/null +++ b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/button.module.css @@ -0,0 +1,5 @@ +.button { + display: inline-block; + border: 1px solid black; + background: white; +} diff --git a/test/e2e/app-dir/app-css/app/css/css-conflict-layers/layout.js b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/layout.js new file mode 100644 index 0000000000000..ee4defadf9a15 --- /dev/null +++ b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/layout.js @@ -0,0 +1,12 @@ +import { BlueButton } from './blue-button' + +export default function ServerLayout({ children }) { + return ( + <> +
+ Blue Button: +
+ {children} + + ) +} diff --git a/test/e2e/app-dir/app-css/app/css/css-conflict-layers/page.css b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/page.css new file mode 100644 index 0000000000000..3a94c07339f95 --- /dev/null +++ b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/page.css @@ -0,0 +1,3 @@ +body { + font-size: large; +} diff --git a/test/e2e/app-dir/app-css/app/css/css-conflict-layers/page.js b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/page.js new file mode 100644 index 0000000000000..287988b3eef91 --- /dev/null +++ b/test/e2e/app-dir/app-css/app/css/css-conflict-layers/page.js @@ -0,0 +1,13 @@ +import './page.css' + +import { Button } from './button' + +export default function Page() { + return ( + <> +
+ Button:
+ + ) +} diff --git a/test/e2e/app-dir/app-css/index.test.ts b/test/e2e/app-dir/app-css/index.test.ts index 1b92b2aafbeb4..74ea957e387a5 100644 --- a/test/e2e/app-dir/app-css/index.test.ts +++ b/test/e2e/app-dir/app-css/index.test.ts @@ -347,6 +347,24 @@ createNextDescribe( ).toBe(1) }) + it('should deduplicate styles on the module level', async () => { + const browser = await next.browser('/css/css-conflict-layers') + await check( + () => + browser.eval( + `window.getComputedStyle(document.querySelector('.btn:not(.btn-blue)')).backgroundColor` + ), + 'rgb(255, 255, 255)' + ) + await check( + () => + browser.eval( + `window.getComputedStyle(document.querySelector('.btn.btn-blue')).backgroundColor` + ), + 'rgb(0, 0, 255)' + ) + }) + it('should only include the same style once in the flight data', async () => { const initialHtml = await next.render('/css/css-duplicate-2/server')