Skip to content

Commit

Permalink
Remove cssFiles field (#50610)
Browse files Browse the repository at this point in the history
This field is no longer needed. Although removing it might cause
unnecessary styles being included that could be tree-shaken potentially,
it solves a more critical issue of missing CSS resources: since now CSS
import can be "hoisted" to its parent entry (e.g. a layout), we can't
simply get all used CSS resources from the "leaf entry" (e.g. a page).
  • Loading branch information
shuding committed Jun 2, 2023
1 parent a0314c0 commit 89a62ac
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 61 deletions.
10 changes: 0 additions & 10 deletions packages/next-swc/crates/next-core/js/src/entry/app-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,6 @@ async function runOperation(renderData: RenderData) {
if (prop === 'clientModules') {
return clientModulesProxy
}
if (prop === 'cssFiles') {
return new Proxy({} as any, cssFilesProxyMethods)
}
},
}
}
Expand All @@ -219,13 +216,6 @@ async function runOperation(renderData: RenderData) {
return needed
}
}
const cssFilesProxyMethods = {
get(_target: any, prop: string) {
const cssChunks = JSON.parse(prop)
// TODO(WEB-856) subscribe to changes
return cssChunks.map(toPath)
},
}
const cssImportProxyMethods = {
get(_target: any, prop: string) {
const cssChunks = JSON.parse(prop.replace(/\.js$/, ''))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
CLIENT_REFERENCE_MANIFEST,
SYSTEM_ENTRYPOINTS,
} from '../../../shared/lib/constants'
import { relative, sep } from 'path'
import { relative } from 'path'
import { isClientComponentEntryModule, isCSSMod } from '../loaders/utils'
import { getProxiedPluginState } from '../../build-context'

Expand Down Expand Up @@ -69,9 +69,6 @@ export type ClientReferenceManifest = {
edgeSSRModuleMapping: {
[moduleId: string]: ManifestNode
}
cssFiles: {
[entryPathWithoutExtension: string]: string[]
}
}

export type ClientCSSReferenceManifest = {
Expand Down Expand Up @@ -132,7 +129,6 @@ export class ClientReferenceManifestPlugin {
const manifest: ClientReferenceManifest = {
ssrModuleMapping: {},
edgeSSRModuleMapping: {},
cssFiles: {},
clientModules: {},
}

Expand Down Expand Up @@ -384,37 +380,6 @@ export class ClientReferenceManifestPlugin {
}
}
})

const entryCSSFiles: { [key: string]: string[] } = manifest.cssFiles || {}

const addCSSFilesToEntry = (
files: string[],
entryName: string | undefined | null
) => {
if (entryName?.startsWith('app/')) {
// The `key` here should be the absolute file path but without extension.
// We need to replace the separator in the entry name to match the system separator.
const key =
this.appDir +
entryName
.slice(3)
.replace(/\//g, sep)
.replace(/\.[^\\/.]+$/, '')
entryCSSFiles[key] = files.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.cssFiles = entryCSSFiles
})

const file = 'server/' + CLIENT_REFERENCE_MANIFEST
Expand Down
19 changes: 6 additions & 13 deletions packages/next/src/server/app-render/get-css-inlined-link-tags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,8 @@ export function getCssInlinedLinkTags(
const layoutOrPageCssModules = serverCSSManifest.cssImports[filePath]

const filePathWithoutExt = filePath.replace(/(\.[A-Za-z0-9]+)+$/, '')
const cssFilesForEntry = new Set(
clientReferenceManifest.cssFiles?.[filePathWithoutExt] || []
)

if (!layoutOrPageCssModules || !cssFilesForEntry.size) {
if (!layoutOrPageCssModules) {
return []
}
const chunks = new Set<string>()
Expand All @@ -49,15 +46,11 @@ export function getCssInlinedLinkTags(
]
if (modData) {
for (const chunk of modData.chunks) {
// If the current entry in the final tree-shaked bundle has that CSS
// chunk, it means that it's actually used. We should include it.
if (cssFilesForEntry.has(chunk)) {
chunks.add(chunk)
// This might be a new layout, and to make it more efficient and
// not introducing another loop, we mutate the set directly.
if (collectNewCSSImports) {
injectedCSS.add(mod)
}
chunks.add(chunk)
// This might be a new layout, and to make it more efficient and
// not introducing another loop, we mutate the set directly.
if (collectNewCSSImports) {
injectedCSS.add(mod)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/app-dir/app-css/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ createNextDescribe(
})

describe('multiple entries', () => {
it('should only inject the same style once if used by different layers', async () => {
it.skip('should only inject the same style once if used by different layers', async () => {
const browser = await next.browser('/css/css-duplicate-2/client')
expect(
await browser.eval(
Expand Down Expand Up @@ -375,7 +375,7 @@ createNextDescribe(
).toBe(3)
})

it('should only load chunks for the css module that is used by the specific entrypoint', async () => {
it.skip('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')

Expand Down

0 comments on commit 89a62ac

Please sign in to comment.