From f12788dee8088f4faed3a86c2a51295f7720a070 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Thu, 25 Aug 2022 17:40:16 +0100 Subject: [PATCH] HMR for client CSS imports (#39916) Follow-up to #39758, this PR makes sure that CSS imports (both global and CSS modules) from client components are not handled by mini-css-extract's HMR logic. Instead, we trigger a server component update and let the client to refetch the RSC payload. However, we are still leveraging the mini-css-extract plugin to emit CSS assets. So in this PR we add a new pitch loader to calculate the original file hash, but replace the final content to eliminate HMR logic but only keep the hash (so hot reloader can keep tracking that). ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples) --- .../build/webpack/config/blocks/css/index.ts | 305 +++++++++++------- .../config/blocks/css/loaders/client.ts | 6 +- .../loaders/next-flight-css-dev-loader.ts | 23 +- .../webpack/plugins/flight-manifest-plugin.ts | 13 +- packages/next/server/app-render.tsx | 7 +- packages/next/server/dev/hot-reloader.ts | 31 +- .../app/app/css/css-client/client-page.css | 2 +- test/e2e/app-dir/app/app/css/layout.server.js | 2 +- 8 files changed, 240 insertions(+), 149 deletions(-) diff --git a/packages/next/build/webpack/config/blocks/css/index.ts b/packages/next/build/webpack/config/blocks/css/index.ts index 2585a2202262c3e..011615134b04fbb 100644 --- a/packages/next/build/webpack/config/blocks/css/index.ts +++ b/packages/next/build/webpack/config/blocks/css/index.ts @@ -201,59 +201,121 @@ export const css = curry(async function css( // CSS Modules support must be enabled on the server and client so the class // names are available for SSR or Prerendering. - fns.push( - loader({ - oneOf: [ - markRemovable({ - // CSS Modules should never have side effects. This setting will - // allow unused CSS to be removed from the production build. - // We ensure this by disallowing `:global()` CSS at the top-level - // via the `pure` mode in `css-loader`. - sideEffects: false, - // CSS Modules are activated via this specific extension. - test: regexCssModules, - // CSS Modules are only supported in the user's application. We're - // not yet allowing CSS imports _within_ `node_modules`. - issuer: { - and: [ - { - or: [ctx.rootDirectory, regexClientEntry], - }, + if (ctx.experimental.appDir && !ctx.isProduction) { + fns.push( + loader({ + oneOf: [ + markRemovable({ + // CSS Modules should never have side effects. This setting will + // allow unused CSS to be removed from the production build. + // We ensure this by disallowing `:global()` CSS at the top-level + // via the `pure` mode in `css-loader`. + sideEffects: false, + // CSS Modules are activated via this specific extension. + test: regexCssModules, + // CSS Modules are only supported in the user's application. We're + // not yet allowing CSS imports _within_ `node_modules`. + issuer: { + and: [ + { + or: [ctx.rootDirectory, regexClientEntry], + }, + ], + not: [/node_modules/], + }, + use: [ + require.resolve('../../../loaders/next-flight-css-dev-loader'), + ...getCssModuleLoader(ctx, lazyPostCSSInitializer), ], - not: [/node_modules/], - }, - use: getCssModuleLoader(ctx, lazyPostCSSInitializer), - }), - ], - }) - ) - fns.push( - loader({ - oneOf: [ - // Opt-in support for Sass (using .scss or .sass extensions). - markRemovable({ - // Sass Modules should never have side effects. This setting will - // allow unused Sass to be removed from the production build. - // We ensure this by disallowing `:global()` Sass at the top-level - // via the `pure` mode in `css-loader`. - sideEffects: false, - // Sass Modules are activated via this specific extension. - test: regexSassModules, - // Sass Modules are only supported in the user's application. We're - // not yet allowing Sass imports _within_ `node_modules`. - issuer: { - and: [ctx.rootDirectory], - not: [/node_modules/], - }, - use: getCssModuleLoader( - ctx, - lazyPostCSSInitializer, - sassPreprocessors - ), - }), - ], - }) - ) + }), + ], + }) + ) + fns.push( + loader({ + oneOf: [ + // Opt-in support for Sass (using .scss or .sass extensions). + markRemovable({ + // Sass Modules should never have side effects. This setting will + // allow unused Sass to be removed from the production build. + // We ensure this by disallowing `:global()` Sass at the top-level + // via the `pure` mode in `css-loader`. + sideEffects: false, + // Sass Modules are activated via this specific extension. + test: regexSassModules, + // Sass Modules are only supported in the user's application. We're + // not yet allowing Sass imports _within_ `node_modules`. + issuer: { + and: [ctx.rootDirectory], + not: [/node_modules/], + }, + use: [ + require.resolve('../../../loaders/next-flight-css-dev-loader'), + ...getCssModuleLoader( + ctx, + lazyPostCSSInitializer, + sassPreprocessors + ), + ], + }), + ], + }) + ) + } else { + fns.push( + loader({ + oneOf: [ + markRemovable({ + // CSS Modules should never have side effects. This setting will + // allow unused CSS to be removed from the production build. + // We ensure this by disallowing `:global()` CSS at the top-level + // via the `pure` mode in `css-loader`. + sideEffects: false, + // CSS Modules are activated via this specific extension. + test: regexCssModules, + // CSS Modules are only supported in the user's application. We're + // not yet allowing CSS imports _within_ `node_modules`. + issuer: { + and: [ + { + or: [ctx.rootDirectory, regexClientEntry], + }, + ], + not: [/node_modules/], + }, + use: getCssModuleLoader(ctx, lazyPostCSSInitializer), + }), + ], + }) + ) + fns.push( + loader({ + oneOf: [ + // Opt-in support for Sass (using .scss or .sass extensions). + markRemovable({ + // Sass Modules should never have side effects. This setting will + // allow unused Sass to be removed from the production build. + // We ensure this by disallowing `:global()` Sass at the top-level + // via the `pure` mode in `css-loader`. + sideEffects: false, + // Sass Modules are activated via this specific extension. + test: regexSassModules, + // Sass Modules are only supported in the user's application. We're + // not yet allowing Sass imports _within_ `node_modules`. + issuer: { + and: [ctx.rootDirectory], + not: [/node_modules/], + }, + use: getCssModuleLoader( + ctx, + lazyPostCSSInitializer, + sassPreprocessors + ), + }), + ], + }) + ) + } if (!ctx.experimental.appDir) { // Throw an error for CSS Modules used outside their supported scope @@ -280,6 +342,7 @@ export const css = curry(async function css( loader({ oneOf: [ markRemovable({ + sideEffects: true, test: [regexCssGlobal, regexSassGlobal], use: require.resolve( '../../../loaders/next-flight-css-dev-loader' @@ -301,38 +364,7 @@ export const css = curry(async function css( ) } } else { - fns.push( - loader({ - oneOf: [ - markRemovable({ - // A global CSS import always has side effects. Webpack will tree - // shake the CSS without this option if the issuer claims to have - // no side-effects. - // See https://github.com/webpack/webpack/issues/6571 - sideEffects: true, - test: regexCssGlobal, - // We only allow Global CSS to be imported anywhere in the - // application if it comes from node_modules. This is a best-effort - // heuristic that makes a safety trade-off for better - // interoperability with npm packages that require CSS. Without - // this ability, the component's CSS would have to be included for - // the entire app instead of specific page where it's required. - include: { and: [/node_modules/] }, - // Global CSS is only supported in the user's application, not in - // node_modules. - issuer: ctx.experimental.craCompat - ? undefined - : { - and: [ctx.rootDirectory], - not: [/node_modules/], - }, - use: getGlobalCssLoader(ctx, lazyPostCSSInitializer), - }), - ], - }) - ) - - if (ctx.customAppFile) { + if (ctx.experimental.appDir) { fns.push( loader({ oneOf: [ @@ -343,8 +375,10 @@ export const css = curry(async function css( // See https://github.com/webpack/webpack/issues/6571 sideEffects: true, test: regexCssGlobal, - issuer: { and: [ctx.customAppFile] }, - use: getGlobalCssLoader(ctx, lazyPostCSSInitializer), + use: [ + require.resolve('../../../loaders/next-flight-css-dev-loader'), + ...getGlobalCssLoader(ctx, lazyPostCSSInitializer), + ], }), ], }) @@ -353,25 +387,17 @@ export const css = curry(async function css( loader({ oneOf: [ markRemovable({ - // A global Sass import always has side effects. Webpack will tree - // shake the Sass without this option if the issuer claims to have - // no side-effects. - // See https://github.com/webpack/webpack/issues/6571 - sideEffects: true, - test: regexSassGlobal, - issuer: { and: [ctx.customAppFile] }, - use: getGlobalCssLoader( - ctx, - lazyPostCSSInitializer, - sassPreprocessors - ), + sideEffects: false, + test: regexCssModules, + use: [ + require.resolve('../../../loaders/next-flight-css-dev-loader'), + ...getCssModuleLoader(ctx, lazyPostCSSInitializer), + ], }), ], }) ) - } - - if (ctx.experimental.appDir) { + } else { fns.push( loader({ oneOf: [ @@ -382,36 +408,65 @@ export const css = curry(async function css( // See https://github.com/webpack/webpack/issues/6571 sideEffects: true, test: regexCssGlobal, - issuer: { - and: [ - { - or: [ - { and: [ctx.rootDirectory, /\.(js|mjs|jsx|ts|tsx)$/] }, - regexClientEntry, - ], + // We only allow Global CSS to be imported anywhere in the + // application if it comes from node_modules. This is a best-effort + // heuristic that makes a safety trade-off for better + // interoperability with npm packages that require CSS. Without + // this ability, the component's CSS would have to be included for + // the entire app instead of specific page where it's required. + include: { and: [/node_modules/] }, + // Global CSS is only supported in the user's application, not in + // node_modules. + issuer: ctx.experimental.craCompat + ? undefined + : { + and: [ctx.rootDirectory], + not: [/node_modules/], }, - ], - }, use: getGlobalCssLoader(ctx, lazyPostCSSInitializer), }), ], }) ) - fns.push( - loader({ - oneOf: [ - markRemovable({ - sideEffects: false, - test: regexCssModules, - issuer: { - and: [ctx.rootDirectory, /\.(js|mjs|jsx|ts|tsx)$/], - or: [regexClientEntry], - }, - use: getCssModuleLoader(ctx, lazyPostCSSInitializer), - }), - ], - }) - ) + + if (ctx.customAppFile) { + fns.push( + loader({ + oneOf: [ + markRemovable({ + // A global CSS import always has side effects. Webpack will tree + // shake the CSS without this option if the issuer claims to have + // no side-effects. + // See https://github.com/webpack/webpack/issues/6571 + sideEffects: true, + test: regexCssGlobal, + issuer: { and: [ctx.customAppFile] }, + use: getGlobalCssLoader(ctx, lazyPostCSSInitializer), + }), + ], + }) + ) + fns.push( + loader({ + oneOf: [ + markRemovable({ + // A global Sass import always has side effects. Webpack will tree + // shake the Sass without this option if the issuer claims to have + // no side-effects. + // See https://github.com/webpack/webpack/issues/6571 + sideEffects: true, + test: regexSassGlobal, + issuer: { and: [ctx.customAppFile] }, + use: getGlobalCssLoader( + ctx, + lazyPostCSSInitializer, + sassPreprocessors + ), + }), + ], + }) + ) + } } } diff --git a/packages/next/build/webpack/config/blocks/css/loaders/client.ts b/packages/next/build/webpack/config/blocks/css/loaders/client.ts index 427898caf5da057..78d662643b46cc0 100644 --- a/packages/next/build/webpack/config/blocks/css/loaders/client.ts +++ b/packages/next/build/webpack/config/blocks/css/loaders/client.ts @@ -40,8 +40,10 @@ export function getClientStyleLoader({ const MiniCssExtractPlugin = require('../../../../plugins/mini-css-extract-plugin').default return { - // @ts-ignore: TODO: remove when webpack 5 is stable loader: MiniCssExtractPlugin.loader, - options: { publicPath: `${assetPrefix}/_next/`, esModule: false }, + options: { + publicPath: `${assetPrefix}/_next/`, + esModule: false, + }, } } diff --git a/packages/next/build/webpack/loaders/next-flight-css-dev-loader.ts b/packages/next/build/webpack/loaders/next-flight-css-dev-loader.ts index d2f9ab52146688b..30947cd00dd62a5 100644 --- a/packages/next/build/webpack/loaders/next-flight-css-dev-loader.ts +++ b/packages/next/build/webpack/loaders/next-flight-css-dev-loader.ts @@ -4,13 +4,26 @@ * inside a comment. */ -const NextServerCSSLoader = function (this: any, source: string | Buffer) { +export function pitch(this: any) { + const content = this.fs.readFileSync(this.resource) + this.data.__checksum = ( + typeof content === 'string' ? Buffer.from(content) : content + ).toString('hex') +} + +const NextServerCSSLoader = function (this: any, content: string) { this.cacheable && this.cacheable() - return `export default "${(typeof source === 'string' - ? Buffer.from(source) - : source - ).toString('hex')}"` + const isCSSModule = this.resource.match(/\.module\.css$/) + if (isCSSModule) { + return ( + content + + '\nmodule.exports.__checksum = ' + + JSON.stringify(this.data.__checksum) + ) + } + + return `export default ${JSON.stringify(this.data.__checksum)}` } export default NextServerCSSLoader diff --git a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts index 6b4d0f328bfca5e..d300c27d7209da6 100644 --- a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts @@ -113,7 +113,7 @@ export class FlightManifestPlugin { const dev = this.dev compilation.chunkGroups.forEach((chunkGroup) => { - const cssResourcesInChunkGroup: string[] = [] + const cssResourcesInChunkGroup = new Set() let entryFilepath: string = '' function recordModule( @@ -170,17 +170,10 @@ export class FlightManifestPlugin { chunks, }, } - moduleIdMapping[id] = moduleIdMapping[id] || {} - moduleIdMapping[id]['default'] = { - id: ssrNamedModuleId, - name: 'default', - chunks, - } - manifest.__ssr_module_mapping__ = moduleIdMapping } if (chunkGroup.name) { - cssResourcesInChunkGroup.push(resource) + cssResourcesInChunkGroup.add(resource) } return @@ -297,7 +290,7 @@ export class FlightManifestPlugin { const clientCSSManifest: any = manifest.__client_css_manifest__ || {} if (entryFilepath) { - clientCSSManifest[entryFilepath] = cssResourcesInChunkGroup + clientCSSManifest[entryFilepath] = Array.from(cssResourcesInChunkGroup) } manifest.__client_css_manifest__ = clientCSSManifest }) diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index f61757c58ee2e06..774f89918d2b9b5 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -393,8 +393,11 @@ function getCssInlinedLinkTags( const chunks = new Set() for (const css of layoutOrPageCss) { - for (const chunk of serverComponentManifest[css].default.chunks) { - chunks.add(chunk) + const mod = serverComponentManifest[css] + if (mod) { + for (const chunk of mod.default.chunks) { + chunks.add(chunk) + } } } diff --git a/packages/next/server/dev/hot-reloader.ts b/packages/next/server/dev/hot-reloader.ts index 6bb16c5e975c0ee..76aa4510599635f 100644 --- a/packages/next/server/dev/hot-reloader.ts +++ b/packages/next/server/dev/hot-reloader.ts @@ -726,9 +726,12 @@ export default class HotReloader { const changedClientPages = new Set() const changedServerPages = new Set() const changedEdgeServerPages = new Set() + const changedCSSImportPages = new Set() + const prevClientPageHashes = new Map() const prevServerPageHashes = new Map() const prevEdgeServerPageHashes = new Map() + const prevCSSImportModuleHashes = new Map() const trackPageChanges = (pageHashMap: Map, changedItems: Set) => @@ -746,6 +749,7 @@ export default class HotReloader { const modsIterable: any = stats.chunkGraph.getChunkModulesIterable(chunk) + let hasCSSModuleChanges = false let chunksHash = new StringXor() modsIterable.forEach((mod: any) => { @@ -771,6 +775,21 @@ export default class HotReloader { chunk.runtime ) chunksHash.add(hash) + + // Both CSS import changes from server and client + // components are tracked. + if ( + key.startsWith('app/') && + mod.resource?.endsWith('.css') + ) { + const prevHash = prevCSSImportModuleHashes.get( + mod.resource + ) + if (prevHash && prevHash !== hash) { + hasCSSModuleChanges = true + } + prevCSSImportModuleHashes.set(mod.resource, hash) + } } }) const prevHash = pageHashMap.get(key) @@ -780,6 +799,10 @@ export default class HotReloader { changedItems.add(key) } pageHashMap.set(key, curHash) + + if (hasCSSModuleChanges) { + changedCSSImportPages.add(key) + } } }) } @@ -857,18 +880,20 @@ export default class HotReloader { changedServerPages, changedClientPages ) - const serverComponentChanges = serverOnlyChanges.filter((key) => - key.startsWith('app/') - ) + const serverComponentChanges = serverOnlyChanges + .filter((key) => key.startsWith('app/')) + .concat(Array.from(changedCSSImportPages)) const pageChanges = serverOnlyChanges.filter((key) => key.startsWith('pages/') ) const middlewareChanges = Array.from(changedEdgeServerPages).filter( (name) => isMiddlewareFilename(name) ) + changedClientPages.clear() changedServerPages.clear() changedEdgeServerPages.clear() + changedCSSImportPages.clear() if (middlewareChanges.length > 0) { this.send({ diff --git a/test/e2e/app-dir/app/app/css/css-client/client-page.css b/test/e2e/app-dir/app/app/css/css-client/client-page.css index 4b2edc961d9d222..f3e551c05319ffe 100644 --- a/test/e2e/app-dir/app/app/css/css-client/client-page.css +++ b/test/e2e/app-dir/app/app/css/css-client/client-page.css @@ -2,5 +2,5 @@ h1 { color: red !important; } h1::after { - content: ' (from css-client!!)'; + content: ' (from css-client!!!!)'; } diff --git a/test/e2e/app-dir/app/app/css/layout.server.js b/test/e2e/app-dir/app/app/css/layout.server.js index da0f5ee6228a47f..3b1bdd72bbb0a39 100644 --- a/test/e2e/app-dir/app/app/css/layout.server.js +++ b/test/e2e/app-dir/app/app/css/layout.server.js @@ -5,7 +5,7 @@ import styles from './style.module.css' export default function ServerLayout({ children }) { return ( <> -
+
Server Layout: CSS Modules
Server Layout: Global CSS