Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove cssFiles field #50610

Merged
merged 2 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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