Skip to content

Commit

Permalink
fix(css): correctly set manifest source name and emit CSS file (#14945)
Browse files Browse the repository at this point in the history
  • Loading branch information
sapphi-red committed Nov 11, 2023
1 parent 9a57916 commit 28ccede
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 34 deletions.
18 changes: 8 additions & 10 deletions packages/vite/src/node/plugins/css.ts
Expand Up @@ -70,6 +70,7 @@ import {
renderAssetUrlInJS,
} from './asset'
import type { ESBuildOptions } from './esbuild'
import { getChunkOriginalFileName } from './manifest'

// const debug = createDebugger('vite:css')

Expand Down Expand Up @@ -610,15 +611,13 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
}
if (opts.format === 'es' || opts.format === 'cjs') {
const isEntry = chunk.isEntry && isPureCssChunk
const cssAssetName = normalizePath(
!isEntry && chunk.facadeModuleId
? path.relative(config.root, chunk.facadeModuleId)
: chunk.name,
const cssAssetName = ensureFileExt(chunk.name, '.css')
const originalFilename = getChunkOriginalFileName(
chunk,
config.root,
opts.format,
)

const lang = path.extname(cssAssetName).slice(1)
const cssFileName = ensureFileExt(cssAssetName, '.css')

chunkCSS = resolveAssetUrlsInCss(chunkCSS, cssAssetName)

const previousTask = emitTasks[emitTasks.length - 1]
Expand All @@ -639,14 +638,13 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {

// emit corresponding css file
const referenceId = this.emitFile({
name: path.basename(cssFileName),
name: cssAssetName,
type: 'asset',
source: chunkCSS,
})
const originalName = isPreProcessor(lang) ? cssAssetName : cssFileName
generatedAssets
.get(config)!
.set(referenceId, { originalName, isEntry })
.set(referenceId, { originalName: originalFilename, isEntry })
chunk.viteMetadata!.importedCss.add(this.getFileName(referenceId))

if (emitTasksLength === emitTasks.length) {
Expand Down
43 changes: 29 additions & 14 deletions packages/vite/src/node/plugins/manifest.ts
@@ -1,5 +1,10 @@
import path from 'node:path'
import type { OutputAsset, OutputChunk } from 'rollup'
import type {
InternalModuleFormat,
OutputAsset,
OutputChunk,
RenderedChunk,
} from 'rollup'
import jsonStableStringify from 'json-stable-stringify'
import type { ResolvedConfig } from '..'
import type { Plugin } from '../plugin'
Expand Down Expand Up @@ -34,19 +39,7 @@ export function manifestPlugin(config: ResolvedConfig): Plugin {

generateBundle({ format }, bundle) {
function getChunkName(chunk: OutputChunk) {
if (chunk.facadeModuleId) {
let name = normalizePath(
path.relative(config.root, chunk.facadeModuleId),
)
if (format === 'system' && !chunk.name.includes('-legacy')) {
const ext = path.extname(name)
const endPos = ext.length !== 0 ? -ext.length : undefined
name = name.slice(0, endPos) + `-legacy` + ext
}
return name.replace(/\0/g, '')
} else {
return `_` + path.basename(chunk.fileName)
}
return getChunkOriginalFileName(chunk, config.root, format)
}

function getInternalImports(imports: string[]): string[] {
Expand Down Expand Up @@ -133,6 +126,10 @@ export function manifestPlugin(config: ResolvedConfig): Plugin {
const assetMeta = fileNameToAssetMeta.get(chunk.fileName)
const src = assetMeta?.originalName ?? chunk.name
const asset = createAsset(chunk, src, assetMeta?.isEntry)

// If JS chunk and asset chunk are both generated from the same source file,
// prioritize JS chunk as it contains more information
if (manifest[src]?.file.endsWith('.js')) continue
manifest[src] = asset
fileNameToAsset.set(chunk.fileName, asset)
}
Expand Down Expand Up @@ -165,3 +162,21 @@ export function manifestPlugin(config: ResolvedConfig): Plugin {
},
}
}

export function getChunkOriginalFileName(
chunk: OutputChunk | RenderedChunk,
root: string,
format: InternalModuleFormat,
): string {
if (chunk.facadeModuleId) {
let name = normalizePath(path.relative(root, chunk.facadeModuleId))
if (format === 'system' && !chunk.name.includes('-legacy')) {
const ext = path.extname(name)
const endPos = ext.length !== 0 ? -ext.length : undefined
name = name.slice(0, endPos) + `-legacy` + ext
}
return name.replace(/\0/g, '')
} else {
return `_` + path.basename(chunk.fileName)
}
}
Expand Up @@ -35,22 +35,24 @@ describe.runIf(isBuild)('build', () => {
const manifest = readManifest('dev')
const htmlEntry = manifest['index.html']
const cssAssetEntry = manifest['global.css']
const pcssAssetEntry = manifest['foo.pcss']
const scssAssetEntry = manifest['nested/blue.scss']
const imgAssetEntry = manifest['../images/logo.png']
const dirFooAssetEntry = manifest['../dynamic/foo.css'] // '\\' should not be used even on windows
const dirFooAssetEntry = manifest['../../dir/foo.css']
expect(htmlEntry.css.length).toEqual(1)
expect(htmlEntry.assets.length).toEqual(1)
expect(cssAssetEntry?.file).not.toBeUndefined()
expect(cssAssetEntry?.isEntry).toEqual(true)
expect(pcssAssetEntry?.file).not.toBeUndefined()
expect(pcssAssetEntry?.isEntry).toEqual(true)
expect(scssAssetEntry?.file).not.toBeUndefined()
expect(scssAssetEntry?.src).toEqual('nested/blue.scss')
expect(scssAssetEntry?.isEntry).toEqual(true)
expect(imgAssetEntry?.file).not.toBeUndefined()
expect(imgAssetEntry?.isEntry).toBeUndefined()
expect(dirFooAssetEntry).not.toBeUndefined()
expect(dirFooAssetEntry).not.toBeUndefined() // '\\' should not be used even on windows
// use the entry name
expect(manifest['bar.css']).not.toBeUndefined()
expect(manifest['foo.css']).toBeUndefined()
expect(dirFooAssetEntry.file).toMatch('assets/bar-')
})
})

Expand Down
2 changes: 1 addition & 1 deletion playground/backend-integration/dir/foo.css
@@ -1,3 +1,3 @@
.entry-name-foo {
.windows-path-foo {
color: blue;
}
3 changes: 0 additions & 3 deletions playground/backend-integration/frontend/dynamic/foo.css

This file was deleted.

1 change: 0 additions & 1 deletion playground/backend-integration/frontend/dynamic/foo.ts

This file was deleted.

3 changes: 3 additions & 0 deletions playground/backend-integration/frontend/entrypoints/foo.pcss
@@ -0,0 +1,3 @@
.foo_pcss {
color: blue;
}
@@ -1,5 +1,4 @@
import 'vite/modulepreload-polyfill'
import('../dynamic/foo') // should be dynamic import to split chunks

export const colorClass = 'text-black'

Expand Down
7 changes: 7 additions & 0 deletions playground/css/__tests__/css.spec.ts
Expand Up @@ -508,3 +508,10 @@ test('async css order with css modules', async () => {
test('@import scss', async () => {
expect(await getColor('.at-import-scss')).toBe('red')
})

test.runIf(isBuild)('manual chunk path', async () => {
// assert that the manual-chunk css is output in the directory specified in manualChunk (#12072)
expect(
findAssetFile(/dir\/dir2\/manual-chunk-[-\w]{8}\.css$/),
).not.toBeUndefined()
})
1 change: 1 addition & 0 deletions playground/css/main.js
Expand Up @@ -4,6 +4,7 @@ import './sugarss.sss'
import './sass.scss'
import './less.less'
import './stylus.styl'
import './manual-chunk.css'

import rawCss from './raw-imported.css?raw'
text('.raw-imported-css', rawCss)
Expand Down
3 changes: 3 additions & 0 deletions playground/css/manual-chunk.css
@@ -0,0 +1,3 @@
.manual-chunk {
color: blue;
}
9 changes: 9 additions & 0 deletions playground/css/vite.config.js
Expand Up @@ -12,6 +12,15 @@ globalThis.location = new URL('http://localhost/')
export default defineConfig({
build: {
cssTarget: 'chrome61',
rollupOptions: {
output: {
manualChunks(id) {
if (id.includes('manual-chunk.css')) {
return 'dir/dir2/manual-chunk'
}
},
},
},
},
esbuild: {
logOverride: {
Expand Down

0 comments on commit 28ccede

Please sign in to comment.