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

fix(css): correctly set manifest source name and emit CSS file #14945

Merged
merged 7 commits into from
Nov 11, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions packages/vite/src/node/plugins/css.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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)
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.foo_pcss {
color: blue;
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.manual-chunk {
color: blue;
}
9 changes: 9 additions & 0 deletions playground/css/vite.config.js
Original file line number Diff line number Diff line change
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