From 5bbc0029644cee8a92b37f8c7c3b7d7b8ff0ecb8 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 24 Aug 2022 18:16:30 +0100 Subject: [PATCH 1/4] support client css imports and hmr --- .../build/webpack/config/blocks/css/index.ts | 305 +++++++++++------- .../config/blocks/css/loaders/client.ts | 6 +- .../loaders/next-flight-css-dev-loader.ts | 23 +- packages/next/server/app-render.tsx | 7 +- packages/next/server/dev/hot-reloader.ts | 33 +- .../app/app/css/css-client/client-foo.css | 2 +- .../app/app/css/css-client/client-layout.css | 2 +- .../app/app/css/css-client/client-page.css | 2 +- .../app/app/css/css-client/page.client.js | 2 +- .../app-dir/app/app/css/css-page/style.css | 2 +- test/e2e/app-dir/app/app/css/layout.server.js | 2 +- 11 files changed, 240 insertions(+), 146 deletions(-) diff --git a/packages/next/build/webpack/config/blocks/css/index.ts b/packages/next/build/webpack/config/blocks/css/index.ts index 2585a2202262..011615134b04 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 427898caf5da..78d662643b46 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 d2f9ab521466..30947cd00dd6 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/server/app-render.tsx b/packages/next/server/app-render.tsx index 012a0682d01a..ea4619d94f6b 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -394,8 +394,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 ffb1e74d8b9a..73af454f98f4 100644 --- a/packages/next/server/dev/hot-reloader.ts +++ b/packages/next/server/dev/hot-reloader.ts @@ -707,9 +707,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) => @@ -727,6 +730,7 @@ export default class HotReloader { const modsIterable: any = stats.chunkGraph.getChunkModulesIterable(chunk) + let hasCSSModuleChanges = false let chunksHash = new StringXor() modsIterable.forEach((mod: any) => { @@ -752,6 +756,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) @@ -761,6 +780,10 @@ export default class HotReloader { changedItems.add(key) } pageHashMap.set(key, curHash) + + if (hasCSSModuleChanges) { + changedCSSImportPages.add(key) + } } }) } @@ -838,18 +861,16 @@ 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() if (middlewareChanges.length > 0) { this.send({ diff --git a/test/e2e/app-dir/app/app/css/css-client/client-foo.css b/test/e2e/app-dir/app/app/css/css-client/client-foo.css index 94ff8d4b4895..a4784e704689 100644 --- a/test/e2e/app-dir/app/app/css/css-client/client-foo.css +++ b/test/e2e/app-dir/app/app/css/css-client/client-foo.css @@ -1,3 +1,3 @@ .foo { - color: blue; + color: violet; } diff --git a/test/e2e/app-dir/app/app/css/css-client/client-layout.css b/test/e2e/app-dir/app/app/css/css-client/client-layout.css index 5bc2906308a0..83480f43792e 100644 --- a/test/e2e/app-dir/app/app/css/css-client/client-layout.css +++ b/test/e2e/app-dir/app/app/css/css-client/client-layout.css @@ -1,3 +1,3 @@ body { - background: cyan; + background: antiquewhite; } 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 4b2edc961d9d..f3e551c05319 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/css-client/page.client.js b/test/e2e/app-dir/app/app/css/css-client/page.client.js index 24df05926ff9..b4220be9933c 100644 --- a/test/e2e/app-dir/app/app/css/css-client/page.client.js +++ b/test/e2e/app-dir/app/app/css/css-client/page.client.js @@ -1,5 +1,5 @@ import './client-page.css' export default function Page() { - return

Page!!!

+ return

Page!

} diff --git a/test/e2e/app-dir/app/app/css/css-page/style.css b/test/e2e/app-dir/app/app/css/css-page/style.css index adc68fa6a4df..3e102df10749 100644 --- a/test/e2e/app-dir/app/app/css/css-page/style.css +++ b/test/e2e/app-dir/app/app/css/css-page/style.css @@ -1,3 +1,3 @@ h1 { - color: red; + text-transform: uppercase; } 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 da0f5ee6228a..3b1bdd72bbb0 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
From 42c03b0223d41ea223314a89fe7a3032d45b7b71 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 24 Aug 2022 18:31:14 +0100 Subject: [PATCH 2/4] fix unintended change --- packages/next/server/dev/hot-reloader.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/next/server/dev/hot-reloader.ts b/packages/next/server/dev/hot-reloader.ts index 73af454f98f4..c9a2c4bf3c0b 100644 --- a/packages/next/server/dev/hot-reloader.ts +++ b/packages/next/server/dev/hot-reloader.ts @@ -864,7 +864,6 @@ export default class HotReloader { const serverComponentChanges = serverOnlyChanges .filter((key) => key.startsWith('app/')) .concat(Array.from(changedCSSImportPages)) - const pageChanges = serverOnlyChanges.filter((key) => key.startsWith('pages/') ) @@ -872,6 +871,11 @@ export default class HotReloader { (name) => isMiddlewareFilename(name) ) + changedClientPages.clear() + changedServerPages.clear() + changedEdgeServerPages.clear() + changedCSSImportPages.clear() + if (middlewareChanges.length > 0) { this.send({ event: 'middlewareChanges', From 037fe9c5b200a85d18819f29d7721bc4c3dba625 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 24 Aug 2022 23:49:42 +0100 Subject: [PATCH 3/4] revert test changes --- test/e2e/app-dir/app/app/css/css-client/client-foo.css | 2 +- test/e2e/app-dir/app/app/css/css-client/client-layout.css | 2 +- test/e2e/app-dir/app/app/css/css-client/page.client.js | 2 +- test/e2e/app-dir/app/app/css/css-page/style.css | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/app-dir/app/app/css/css-client/client-foo.css b/test/e2e/app-dir/app/app/css/css-client/client-foo.css index a4784e704689..94ff8d4b4895 100644 --- a/test/e2e/app-dir/app/app/css/css-client/client-foo.css +++ b/test/e2e/app-dir/app/app/css/css-client/client-foo.css @@ -1,3 +1,3 @@ .foo { - color: violet; + color: blue; } diff --git a/test/e2e/app-dir/app/app/css/css-client/client-layout.css b/test/e2e/app-dir/app/app/css/css-client/client-layout.css index 83480f43792e..5bc2906308a0 100644 --- a/test/e2e/app-dir/app/app/css/css-client/client-layout.css +++ b/test/e2e/app-dir/app/app/css/css-client/client-layout.css @@ -1,3 +1,3 @@ body { - background: antiquewhite; + background: cyan; } diff --git a/test/e2e/app-dir/app/app/css/css-client/page.client.js b/test/e2e/app-dir/app/app/css/css-client/page.client.js index b4220be9933c..24df05926ff9 100644 --- a/test/e2e/app-dir/app/app/css/css-client/page.client.js +++ b/test/e2e/app-dir/app/app/css/css-client/page.client.js @@ -1,5 +1,5 @@ import './client-page.css' export default function Page() { - return

Page!

+ return

Page!!!

} diff --git a/test/e2e/app-dir/app/app/css/css-page/style.css b/test/e2e/app-dir/app/app/css/css-page/style.css index 3e102df10749..adc68fa6a4df 100644 --- a/test/e2e/app-dir/app/app/css/css-page/style.css +++ b/test/e2e/app-dir/app/app/css/css-page/style.css @@ -1,3 +1,3 @@ h1 { - text-transform: uppercase; + color: red; } From 850b87acedcc372255cd391f3a5cf18d26cbd876 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Thu, 25 Aug 2022 16:34:30 +0100 Subject: [PATCH 4/4] fix manifest plugin --- .../build/webpack/plugins/flight-manifest-plugin.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts index e3e721c06f8a..d88d6e7696ec 100644 --- a/packages/next/build/webpack/plugins/flight-manifest-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-manifest-plugin.ts @@ -114,7 +114,7 @@ export class FlightManifestPlugin { const dev = this.dev compilation.chunkGroups.forEach((chunkGroup) => { - const cssResourcesInChunkGroup: string[] = [] + const cssResourcesInChunkGroup = new Set() let entryFilepath: string = '' function recordModule( @@ -171,17 +171,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 @@ -294,7 +287,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 })