Skip to content

Commit

Permalink
Simplify server CSS handling (#51018)
Browse files Browse the repository at this point in the history 
Remove the Server CSS manifest and related logic, and use the chunkGroup
CSS files instead for each entry to inject stylesheet links.

Why was that manifest needed in the first place? When implementing CSS
collection for RSC and nested layout initially, we collect CSS imports
at each layer and create corresponding client entries. But then soon got
hit by the problem of duplication and improper tree-shaking. Two layers
can have the same CSS imported so we solved it in a way that "if an
upper layer imports this module, skip it in child layers". Note that
this is deduped by module, so we need to keep the information of "layer
(entry) → CSS modules" somewhere, in a manifest.

Another reason is that we create the client entry before Webpack
optimizes modules, so we can inject the client entry into the same
compilation. But that means the collected client modules from the server
layer are not properly optimized (DCE). **This is a general issue at the
moment that's not specifically related to CSS, although using that
manifest to collect DCE'd info and join the original collected CSS files
with that info temporarily solved it.** That's why I disabled some tests
related to font CSS collection and want to improve it in a more general
way.

Why is that not needed anymore? Main reason is to keep a good balance
between duplication and number of chunks, and delegate the decision to
Webpack's splitChunks plugin. It is not possible to get to a point of 0
duplication unless we ship every CSS module as a single chunk. And since
in #50406 we made the duplication better but at the chunk asset level,
instead of the original module level.

Prior work: #50406, #50610.
  • Loading branch information
@shuding
shuding authored Jun 14, 2023
1 parent f7c105d commit 9a36f33
Show file tree
Hide file tree
Showing 21 changed files with 117 additions and 341 deletions.
40 changes: 18 additions & 22 deletions packages/next-swc/crates/next-core/js/src/entry/app-renderer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ declare global {

import type { Ipc } from '@vercel/turbopack-node/ipc/index'
import type { IncomingMessage } from 'node:http'
import type {
ClientCSSReferenceManifest,
ClientReferenceManifest,
} from 'next/dist/build/webpack/plugins/flight-manifest-plugin'
import type { ClientReferenceManifest } from 'next/dist/build/webpack/plugins/flight-manifest-plugin'
import type { RenderData } from 'types/turbopack'
import type { RenderOpts } from 'next/dist/server/app-render/types'

Expand Down Expand Up @@ -139,10 +136,11 @@ async function runOperation(renderData: RenderData) {
}

const proxyMethodsNested = (
type: 'ssrModuleMapping' | 'clientModules'
type: 'ssrModuleMapping' | 'clientModules' | 'entryCSSFiles'
): ProxyHandler<
| ClientReferenceManifest['ssrModuleMapping']
| ClientReferenceManifest['clientModules']
| ClientReferenceManifest['entryCSSFiles']
> => {
return {
get(_target, key: string) {
Expand Down Expand Up @@ -170,6 +168,14 @@ async function runOperation(renderData: RenderData) {
chunks: JSON.parse(id)[1],
}
}
if (type === 'entryCSSFiles') {
const cssChunks = JSON.parse(key)
// TODO(WEB-856) subscribe to changes
return {
modules: [],
files: cssChunks.filter(filterAvailable).map(toPath),
}
}
},
}
}
Expand All @@ -183,6 +189,10 @@ async function runOperation(renderData: RenderData) {
{},
proxyMethodsNested('ssrModuleMapping')
)
const entryCSSFilesProxy = new Proxy(
{},
proxyMethodsNested('entryCSSFiles')
)
return {
get(_target: any, prop: string) {
if (prop === 'ssrModuleMapping') {
Expand All @@ -191,6 +201,9 @@ async function runOperation(renderData: RenderData) {
if (prop === 'clientModules') {
return clientModulesProxy
}
if (prop === 'entryCSSFiles') {
return entryCSSFilesProxy
}
},
}
}
Expand All @@ -216,24 +229,8 @@ async function runOperation(renderData: RenderData) {
return needed
}
}
const cssImportProxyMethods = {
get(_target: any, prop: string) {
const cssChunks = JSON.parse(prop.replace(/\.js$/, ''))
// TODO(WEB-856) subscribe to changes

// This return value is passed to proxyMethodsNested for clientModules
return cssChunks
.filter(filterAvailable)
.map(toPath)
.map((chunk: string) => JSON.stringify([chunk, [chunk]]))
},
}
const manifest: ClientReferenceManifest = new Proxy({} as any, proxyMethods())

const serverCSSManifest: ClientCSSReferenceManifest = {
cssImports: new Proxy({} as any, cssImportProxyMethods),
cssModules: {},
}
const req: IncomingMessage = {
url: renderData.originalUrl,
method: renderData.method,
Expand Down Expand Up @@ -275,7 +272,6 @@ async function runOperation(renderData: RenderData) {
pages: ['page.js'],
},
clientReferenceManifest: manifest,
serverCSSManifest,
runtime: 'nodejs',
serverComponents: true,
assetPrefix: '',
Expand Down
9 changes: 0 additions & 9 deletions packages/next/src/build/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ import {
APP_PATHS_MANIFEST,
APP_PATH_ROUTES_MANIFEST,
APP_BUILD_MANIFEST,
FLIGHT_SERVER_CSS_MANIFEST,
RSC_MODULE_TYPES,
NEXT_FONT_MANIFEST,
SUBRESOURCE_INTEGRITY_MANIFEST,
Expand Down Expand Up @@ -893,14 +892,6 @@ export default async function build(
SERVER_DIRECTORY,
CLIENT_REFERENCE_MANIFEST + '.json'
),
path.join(
SERVER_DIRECTORY,
FLIGHT_SERVER_CSS_MANIFEST + '.js'
),
path.join(
SERVER_DIRECTORY,
FLIGHT_SERVER_CSS_MANIFEST + '.json'
),
path.join(
SERVER_DIRECTORY,
SERVER_REFERENCE_MANIFEST + '.js'
Expand Down
2 changes: 0 additions & 2 deletions packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ const edgeSSRLoader: webpack.LoaderDefinitionFunction<EdgeSSRLoaderQuery> =
const prerenderManifest = maybeJSONParse(self.__PRERENDER_MANIFEST)
const reactLoadableManifest = maybeJSONParse(self.__REACT_LOADABLE_MANIFEST)
const rscManifest = maybeJSONParse(self.__RSC_MANIFEST)
const rscCssManifest = maybeJSONParse(self.__RSC_CSS_MANIFEST)
const rscServerManifest = maybeJSONParse(self.__RSC_SERVER_MANIFEST)
const subresourceIntegrityManifest = ${
sriEnabled
Expand All @@ -176,7 +175,6 @@ const edgeSSRLoader: webpack.LoaderDefinitionFunction<EdgeSSRLoaderQuery> =
pagesRenderToHTML,
reactLoadableManifest,
clientReferenceManifest: ${isServerComponent} ? rscManifest : null,
serverCSSManifest: ${isServerComponent} ? rscCssManifest : null,
serverActionsManifest: ${isServerComponent} ? rscServerManifest : null,
subresourceIntegrityManifest,
config: ${stringifiedConfig},
Expand Down
3 changes: 0 additions & 3 deletions packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export function getRender({
pagesRenderToHTML,
clientReferenceManifest,
subresourceIntegrityManifest,
serverCSSManifest,
serverActionsManifest,
config,
buildId,
Expand All @@ -53,7 +52,6 @@ export function getRender({
reactLoadableManifest: ReactLoadableManifest
subresourceIntegrityManifest?: Record<string, string>
clientReferenceManifest?: ClientReferenceManifest
serverCSSManifest: any
serverActionsManifest: any
appServerMod: any
config: NextConfigComplete
Expand Down Expand Up @@ -88,7 +86,6 @@ export function getRender({
supportsDynamicHTML: true,
disableOptimizedLoading: true,
clientReferenceManifest,
serverCSSManifest,
serverActionsManifest,
},
appRenderToHTML,
Expand Down
122 changes: 0 additions & 122 deletions packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type {
ClientComponentImports,
NextFlightClientEntryLoaderOptions,
} from '../loaders/next-flight-client-entry-loader'
import type { ClientCSSReferenceManifest } from './flight-manifest-plugin'

import { webpack } from 'next/dist/compiled/webpack/webpack'
import { stringify } from 'querystring'
Expand All @@ -21,7 +20,6 @@ import {
COMPILER_NAMES,
EDGE_RUNTIME_WEBPACK,
SERVER_REFERENCE_MANIFEST,
FLIGHT_SERVER_CSS_MANIFEST,
} from '../../../shared/lib/constants'
import {
generateActionId,
Expand Down Expand Up @@ -76,10 +74,6 @@ const pluginState = getProxiedPluginState({
}
>,

// Manifest of CSS entry files for server/edge server.
serverCSSManifest: {} as ClientCSSReferenceManifest,
edgeServerCSSManifest: {} as ClientCSSReferenceManifest,

// Mapping of resource path to module id for server/edge server.
serverModuleIds: {} as Record<string, string | number>,
edgeServerModuleIds: {} as Record<string, string | number>,
Expand Down Expand Up @@ -308,7 +302,6 @@ export class ClientReferenceEntryPlugin {
compilation,
entryName: name,
clientComponentImports,
cssImports,
bundlePath,
absolutePagePath: entryRequest,
})
Expand Down Expand Up @@ -475,121 +468,6 @@ export class ClientReferenceEntryPlugin {
return Promise.all(addedClientActionEntryList)
})

// 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<string, Set<string>> = {}

const cssManifest = this.isEdgeServer
? pluginState.edgeServerCSSManifest
: pluginState.serverCSSManifest

function collectModule(entryName: string, mod: any) {
const resource = mod.resource
const modId = resource
if (modId) {
if (isCSSMod(mod)) {
cssImportsForChunk[entryName].add(modId)
}
}
}

compilation.chunkGroups.forEach((chunkGroup: any) => {
chunkGroup.chunks.forEach((chunk: webpack.Chunk) => {
// Here we only track page chunks.
if (!chunk.name) return
if (!chunk.name.endsWith('/page')) return

const entryName = path.join(this.appDir, '..', chunk.name)

if (!cssImportsForChunk[entryName]) {
cssImportsForChunk[entryName] = new Set()
}

const chunkModules = compilation.chunkGraph.getChunkModulesIterable(
chunk
) as Iterable<webpack.NormalModule>
for (const mod of chunkModules) {
collectModule(entryName, mod)

const anyModule = mod as any
if (anyModule.modules) {
anyModule.modules.forEach((concatenatedMod: any) => {
collectModule(entryName, concatenatedMod)
})
}
}

const entryCSSInfo: Record<string, string[]> =
cssManifest.cssModules || {}
entryCSSInfo[entryName] = [...cssImportsForChunk[entryName]]

cssManifest.cssModules = entryCSSInfo
})
})

forEachEntryModule(compilation, ({ name, entryModule }) => {
const clientEntryDependencyMap = collectClientEntryDependencyMap(name)
const tracked = new Set<string>()
const mergedCSSimports: CssImports = {}

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.collectComponentInfoFromDependencies({
entryRequest,
compilation,
dependency: entryDependency,
clientEntryDependencyMap,
})

Object.assign(mergedCSSimports, cssImports)
}

if (!cssManifest.cssImports) cssManifest.cssImports = {}
Object.assign(
cssManifest.cssImports,
deduplicateCSSImportsForEntry(mergedCSSimports)
)
})
})

compilation.hooks.processAssets.tap(
{
name: PLUGIN_NAME,
stage: webpack.Compilation.PROCESS_ASSETS_STAGE_OPTIMIZE_HASH,
},
(assets: webpack.Compilation['assets']) => {
const data: ClientCSSReferenceManifest = {
cssImports: {
...pluginState.serverCSSManifest.cssImports,
...pluginState.edgeServerCSSManifest.cssImports,
},
cssModules: {
...pluginState.serverCSSManifest.cssModules,
...pluginState.edgeServerCSSManifest.cssModules,
},
}
const manifest = JSON.stringify(data, null, this.dev ? 2 : undefined)
assets[`${this.assetPrefix}${FLIGHT_SERVER_CSS_MANIFEST}.json`] =
new sources.RawSource(
manifest
) as unknown as webpack.sources.RawSource
assets[`${this.assetPrefix}${FLIGHT_SERVER_CSS_MANIFEST}.js`] =
new sources.RawSource(
`self.__RSC_CSS_MANIFEST=${JSON.stringify(manifest)}`
) as unknown as webpack.sources.RawSource
}
)

// Invalidate in development to trigger recompilation
const invalidator = getInvalidator(compiler.outputPath)
// Check if any of the entry injections need an invalidation
Expand Down
Loading

0 comments on commit 9a36f33

Please sign in to comment.