From 95fe5a79c434c0078075fc25e244689410447bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= Date: Sat, 18 May 2024 16:22:00 +0900 Subject: [PATCH] fix(css): avoid generating empty JS files when JS files becomes empty but has CSS files imported (#16078) --- packages/vite/src/node/plugins/css.ts | 82 ++++++++++--------- .../__tests__/css-codesplit.spec.ts | 9 +- playground/css-codesplit/async-js.css | 3 + playground/css-codesplit/async-js.js | 2 + playground/css-codesplit/index.html | 1 + playground/css-codesplit/main.js | 1 + .../__tests__/css-no-codesplit.spec.ts | 17 ++++ playground/css-no-codesplit/async-js.css | 3 + playground/css-no-codesplit/async-js.js | 2 + playground/css-no-codesplit/index.html | 5 ++ playground/css-no-codesplit/index.js | 1 + playground/css-no-codesplit/package.json | 12 +++ playground/css-no-codesplit/shared-linked.css | 3 + playground/css-no-codesplit/sub.html | 1 + playground/css-no-codesplit/vite.config.js | 14 ++++ pnpm-lock.yaml | 2 + 16 files changed, 117 insertions(+), 41 deletions(-) create mode 100644 playground/css-codesplit/async-js.css create mode 100644 playground/css-codesplit/async-js.js create mode 100644 playground/css-no-codesplit/__tests__/css-no-codesplit.spec.ts create mode 100644 playground/css-no-codesplit/async-js.css create mode 100644 playground/css-no-codesplit/async-js.js create mode 100644 playground/css-no-codesplit/index.html create mode 100644 playground/css-no-codesplit/index.js create mode 100644 playground/css-no-codesplit/package.json create mode 100644 playground/css-no-codesplit/shared-linked.css create mode 100644 playground/css-no-codesplit/sub.html create mode 100644 playground/css-no-codesplit/vite.config.js diff --git a/packages/vite/src/node/plugins/css.ts b/packages/vite/src/node/plugins/css.ts index 0e97c247c..08c5708de 100644 --- a/packages/vite/src/node/plugins/css.ts +++ b/packages/vite/src/node/plugins/css.ts @@ -549,6 +549,8 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin { async renderChunk(code, chunk, opts) { let chunkCSS = '' + // the chunk is empty if it's a dynamic entry chunk that only contains a CSS import + const isJsChunkEmpty = code === '' && !chunk.isEntry let isPureCssChunk = true const ids = Object.keys(chunk.modules) for (const id of ids) { @@ -561,7 +563,7 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin { isPureCssChunk = false } } - } else { + } else if (!isJsChunkEmpty) { // if the module does not have a style, then it's not a pure css chunk. // this is true because in the `transform` hook above, only modules // that are css gets added to the `styles` map. @@ -723,13 +725,13 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin { } if (chunkCSS) { + if (isPureCssChunk && (opts.format === 'es' || opts.format === 'cjs')) { + // this is a shared CSS-only chunk that is empty. + pureCssChunks.add(chunk) + } + if (config.build.cssCodeSplit) { if (opts.format === 'es' || opts.format === 'cjs') { - if (isPureCssChunk) { - // this is a shared CSS-only chunk that is empty. - pureCssChunks.add(chunk) - } - const isEntry = chunk.isEntry && isPureCssChunk const cssFullAssetName = ensureFileExt(chunk.name, '.css') // if facadeModuleId doesn't exist or doesn't have a CSS extension, @@ -837,6 +839,40 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin { return } + function extractCss() { + let css = '' + const collected = new Set() + const prelimaryNameToChunkMap = new Map( + Object.values(bundle) + .filter((chunk): chunk is OutputChunk => chunk.type === 'chunk') + .map((chunk) => [chunk.preliminaryFileName, chunk]), + ) + + function collect(fileName: string) { + const chunk = bundle[fileName] + if (!chunk || chunk.type !== 'chunk' || collected.has(chunk)) return + collected.add(chunk) + + chunk.imports.forEach(collect) + css += chunkCSSMap.get(chunk.preliminaryFileName) ?? '' + } + + for (const chunkName of chunkCSSMap.keys()) + collect(prelimaryNameToChunkMap.get(chunkName)?.fileName ?? '') + + return css + } + let extractedCss = !hasEmitted && extractCss() + if (extractedCss) { + hasEmitted = true + extractedCss = await finalizeCss(extractedCss, true, config) + this.emitFile({ + name: cssBundleName, + type: 'asset', + source: extractedCss, + }) + } + // remove empty css chunks and their imports if (pureCssChunks.size) { // map each pure css chunk (rendered chunk) to it's corresponding bundle @@ -893,40 +929,6 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin { delete bundle[`${fileName}.map`] }) } - - function extractCss() { - let css = '' - const collected = new Set() - const prelimaryNameToChunkMap = new Map( - Object.values(bundle) - .filter((chunk): chunk is OutputChunk => chunk.type === 'chunk') - .map((chunk) => [chunk.preliminaryFileName, chunk]), - ) - - function collect(fileName: string) { - const chunk = bundle[fileName] - if (!chunk || chunk.type !== 'chunk' || collected.has(chunk)) return - collected.add(chunk) - - chunk.imports.forEach(collect) - css += chunkCSSMap.get(chunk.preliminaryFileName) ?? '' - } - - for (const chunkName of chunkCSSMap.keys()) - collect(prelimaryNameToChunkMap.get(chunkName)?.fileName ?? '') - - return css - } - let extractedCss = !hasEmitted && extractCss() - if (extractedCss) { - hasEmitted = true - extractedCss = await finalizeCss(extractedCss, true, config) - this.emitFile({ - name: cssBundleName, - type: 'asset', - source: extractedCss, - }) - } }, } } diff --git a/playground/css-codesplit/__tests__/css-codesplit.spec.ts b/playground/css-codesplit/__tests__/css-codesplit.spec.ts index 2f7d5ab5f..cc54d865a 100644 --- a/playground/css-codesplit/__tests__/css-codesplit.spec.ts +++ b/playground/css-codesplit/__tests__/css-codesplit.spec.ts @@ -3,6 +3,7 @@ import { findAssetFile, getColor, isBuild, + listAssets, page, readManifest, untilUpdated, @@ -12,6 +13,7 @@ test('should load all stylesheets', async () => { expect(await getColor('h1')).toBe('red') expect(await getColor('h2')).toBe('blue') expect(await getColor('.dynamic')).toBe('green') + expect(await getColor('.async-js')).toBe('blue') expect(await getColor('.chunk')).toBe('magenta') }) @@ -40,7 +42,12 @@ describe.runIf(isBuild)('build', () => { expect(findAssetFile(/style-.*\.js$/)).toBe('') expect(findAssetFile('main.*.js$')).toMatch(`/* empty css`) expect(findAssetFile('other.*.js$')).toMatch(`/* empty css`) - expect(findAssetFile(/async.*\.js$/)).toBe('') + expect(findAssetFile(/async-[-\w]{8}\.js$/)).toBe('') + + const assets = listAssets() + expect(assets).not.toContainEqual( + expect.stringMatching(/async-js-[-\w]{8}\.js$/), + ) }) test('should remove empty chunk, HTML without JS', async () => { diff --git a/playground/css-codesplit/async-js.css b/playground/css-codesplit/async-js.css new file mode 100644 index 000000000..ed61a7f51 --- /dev/null +++ b/playground/css-codesplit/async-js.css @@ -0,0 +1,3 @@ +.async-js { + color: blue; +} diff --git a/playground/css-codesplit/async-js.js b/playground/css-codesplit/async-js.js new file mode 100644 index 000000000..2ce31a1e7 --- /dev/null +++ b/playground/css-codesplit/async-js.js @@ -0,0 +1,2 @@ +// a JS file that becomes an empty file but imports CSS files +import './async-js.css' diff --git a/playground/css-codesplit/index.html b/playground/css-codesplit/index.html index 7d2a4991f..38885fa7c 100644 --- a/playground/css-codesplit/index.html +++ b/playground/css-codesplit/index.html @@ -2,6 +2,7 @@

This should be blue

This should be green

+

This should be blue

This should not be yellow

This should be yellow

diff --git a/playground/css-codesplit/main.js b/playground/css-codesplit/main.js index e548142ad..ec266fa00 100644 --- a/playground/css-codesplit/main.js +++ b/playground/css-codesplit/main.js @@ -9,6 +9,7 @@ import chunkCssUrl from './chunk.css?url' globalThis.__test_chunkCssUrl = chunkCssUrl import('./async.css') +import('./async-js') import('./inline.css?inline').then((css) => { document.querySelector('.dynamic-inline').textContent = css.default diff --git a/playground/css-no-codesplit/__tests__/css-no-codesplit.spec.ts b/playground/css-no-codesplit/__tests__/css-no-codesplit.spec.ts new file mode 100644 index 000000000..5110ef3a7 --- /dev/null +++ b/playground/css-no-codesplit/__tests__/css-no-codesplit.spec.ts @@ -0,0 +1,17 @@ +import { describe, expect, test } from 'vitest' +import { expectWithRetry, getColor, isBuild, listAssets } from '~utils' + +test('should load all stylesheets', async () => { + expect(await getColor('.shared-linked')).toBe('blue') + await expectWithRetry(() => getColor('.async-js')).toBe('blue') +}) + +describe.runIf(isBuild)('build', () => { + test('should remove empty chunk', async () => { + const assets = listAssets() + expect(assets).not.toContainEqual( + expect.stringMatching(/shared-linked-.*\.js$/), + ) + expect(assets).not.toContainEqual(expect.stringMatching(/async-js-.*\.js$/)) + }) +}) diff --git a/playground/css-no-codesplit/async-js.css b/playground/css-no-codesplit/async-js.css new file mode 100644 index 000000000..ed61a7f51 --- /dev/null +++ b/playground/css-no-codesplit/async-js.css @@ -0,0 +1,3 @@ +.async-js { + color: blue; +} diff --git a/playground/css-no-codesplit/async-js.js b/playground/css-no-codesplit/async-js.js new file mode 100644 index 000000000..2ce31a1e7 --- /dev/null +++ b/playground/css-no-codesplit/async-js.js @@ -0,0 +1,2 @@ +// a JS file that becomes an empty file but imports CSS files +import './async-js.css' diff --git a/playground/css-no-codesplit/index.html b/playground/css-no-codesplit/index.html new file mode 100644 index 000000000..e7673c84e --- /dev/null +++ b/playground/css-no-codesplit/index.html @@ -0,0 +1,5 @@ + + + +

shared linked: this should be blue

+

async JS importing CSS: this should be blue

diff --git a/playground/css-no-codesplit/index.js b/playground/css-no-codesplit/index.js new file mode 100644 index 000000000..44b33fda3 --- /dev/null +++ b/playground/css-no-codesplit/index.js @@ -0,0 +1 @@ +import('./async-js') diff --git a/playground/css-no-codesplit/package.json b/playground/css-no-codesplit/package.json new file mode 100644 index 000000000..61d806d3d --- /dev/null +++ b/playground/css-no-codesplit/package.json @@ -0,0 +1,12 @@ +{ + "name": "@vitejs/test-css-no-codesplit", + "private": true, + "version": "0.0.0", + "type": "module", + "scripts": { + "dev": "vite", + "build": "vite build", + "debug": "node --inspect-brk ../../packages/vite/bin/vite", + "preview": "vite preview" + } +} diff --git a/playground/css-no-codesplit/shared-linked.css b/playground/css-no-codesplit/shared-linked.css new file mode 100644 index 000000000..51857a50e --- /dev/null +++ b/playground/css-no-codesplit/shared-linked.css @@ -0,0 +1,3 @@ +.shared-linked { + color: blue; +} diff --git a/playground/css-no-codesplit/sub.html b/playground/css-no-codesplit/sub.html new file mode 100644 index 000000000..f535a771d --- /dev/null +++ b/playground/css-no-codesplit/sub.html @@ -0,0 +1 @@ + diff --git a/playground/css-no-codesplit/vite.config.js b/playground/css-no-codesplit/vite.config.js new file mode 100644 index 000000000..f48d87583 --- /dev/null +++ b/playground/css-no-codesplit/vite.config.js @@ -0,0 +1,14 @@ +import { resolve } from 'node:path' +import { defineConfig } from 'vite' + +export default defineConfig({ + build: { + cssCodeSplit: false, + rollupOptions: { + input: { + index: resolve(__dirname, './index.html'), + sub: resolve(__dirname, './sub.html'), + }, + }, + }, +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 30ebbe595..a58281e55 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -613,6 +613,8 @@ importers: specifier: ^1.24.1 version: 1.24.1 + playground/css-no-codesplit: {} + playground/css-sourcemap: devDependencies: less: