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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(css): remove ?direct in id for postcss process #10514

Merged
merged 5 commits into from Oct 20, 2022
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
134 changes: 83 additions & 51 deletions packages/vite/src/node/__tests__/plugins/css.spec.ts
Expand Up @@ -2,6 +2,7 @@ import fs from 'node:fs'
import path from 'node:path'
import { describe, expect, test, vi } from 'vitest'
import { resolveConfig } from '../../config'
import type { InlineConfig } from '../../config'
import { cssPlugin, cssUrlRE, hoistAtRules } from '../../plugins/css'

describe('search css url function', () => {
Expand Down Expand Up @@ -46,13 +47,17 @@ describe('search css url function', () => {
})
})

describe('css path resolutions', () => {
const mockedProjectPath = path.join(process.cwd(), '/foo/bar/project')
const mockedBarCssRelativePath = '/css/bar.module.css'
const mockedFooCssRelativePath = '/css/foo.module.css'

test('cssmodule compose/from path resolutions', async () => {
Comment on lines -49 to -54
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewer note: There's a lot of change in this test file. The gist of what I did is that I extracted the CSS plugin transform setup for this old test so that I can reuse it for the newly added test.

const config = await resolveConfig(
describe('css modules', () => {
test('css module compose/from path resolutions', async () => {
const mockedProjectPath = path.join(process.cwd(), '/foo/bar/project')
const { transform, resetMock } = await createCssPluginTransform(
{
[path.join(mockedProjectPath, '/css/bar.module.css')]: `\
.bar {
display: block;
background: #f0f;
}`
},
{
resolve: {
alias: [
Expand All @@ -62,57 +67,48 @@ describe('css path resolutions', () => {
}
]
}
},
'serve'
}
)

const { transform, buildStart } = cssPlugin(config)

await buildStart.call({})

const mockFs = vi
.spyOn(fs, 'readFile')
// @ts-ignore vi.spyOn not recognize override `fs.readFile` definition.
.mockImplementationOnce((p, encoding, callback) => {
expect(p).toBe(path.join(mockedProjectPath, mockedBarCssRelativePath))
expect(encoding).toBe('utf-8')
callback(
null,
Buffer.from(`
.bar {
display: block;
background: #f0f;
}
`)
)
})

const { code } = await transform.call(
{
addWatchFile() {
return
}
},
`
const result = await transform(
`\
.foo {
position: fixed;
composes: bar from '@${mockedBarCssRelativePath}';
}
`,
path.join(mockedProjectPath, mockedFooCssRelativePath)
position: fixed;
composes: bar from '@/css/bar.module.css';
}`,
'/css/foo.module.css'
)

expect(code).toBe(`
._bar_soicv_2 {
display: block;
background: #f0f;
}
._foo_sctn3_2 {
position: fixed;
expect(result.code).toBe(
`\
._bar_1csqm_1 {
display: block;
background: #f0f;
}
`)
._foo_86148_1 {
position: fixed;
}`
)

resetMock()
})

mockFs.mockReset()
test('custom generateScopedName', async () => {
const { transform, resetMock } = await createCssPluginTransform(undefined, {
css: {
modules: {
generateScopedName: 'custom__[hash:base64:5]'
}
}
})
const css = `\
.foo {
color: red;
}`
const result1 = await transform(css, '/foo.module.css') // server
const result2 = await transform(css, '/foo.module.css?direct') // client
expect(result1.code).toBe(result2.code)
resetMock()
})
})

Expand Down Expand Up @@ -205,3 +201,39 @@ describe('hoist @ rules', () => {
`)
})
})

async function createCssPluginTransform(
files?: Record<string, string>,
inlineConfig: InlineConfig = {}
) {
const config = await resolveConfig(inlineConfig, 'serve')
const { transform, buildStart } = cssPlugin(config)

// @ts-expect-error
await buildStart.call({})

const mockFs = vi
.spyOn(fs, 'readFile')
// @ts-expect-error vi.spyOn not recognize override `fs.readFile` definition.
.mockImplementationOnce((p, encoding, callback) => {
callback(null, Buffer.from(files?.[p] ?? ''))
})

return {
async transform(code: string, id: string) {
// @ts-expect-error
return await transform.call(
{
addWatchFile() {
return
}
},
code,
id
)
},
resetMock() {
mockFs.mockReset()
}
}
}
6 changes: 4 additions & 2 deletions packages/vite/src/node/plugins/css.ts
Expand Up @@ -42,6 +42,7 @@ import {
normalizePath,
parseRequest,
processSrcSet,
removeDirectQuery,
requireResolveFromRootWithFallback
} from '../utils'
import type { Logger } from '../logger'
Expand Down Expand Up @@ -914,13 +915,14 @@ async function compileCSS(

let postcssResult: PostCSS.Result
try {
const source = removeDirectQuery(id)
// postcss is an unbundled dep and should be lazy imported
postcssResult = await (await import('postcss'))
.default(postcssPlugins)
.process(code, {
...postcssOptions,
to: id,
from: id,
to: source,
from: source,
...(devSourcemap
? {
map: {
Expand Down
4 changes: 4 additions & 0 deletions packages/vite/src/node/utils.ts
Expand Up @@ -303,6 +303,7 @@ export function getPotentialTsSrcPaths(filePath: string): string[] {
}

const importQueryRE = /(\?|&)import=?(?:&|$)/
const directRequestRE = /(\?|&)direct=?(?:&|$)/
const internalPrefixes = [
FS_PREFIX,
VALID_ID_PREFIX,
Expand All @@ -318,6 +319,9 @@ export const isInternalRequest = (url: string): boolean =>
export function removeImportQuery(url: string): string {
return url.replace(importQueryRE, '$1').replace(trailingSeparatorRE, '')
}
export function removeDirectQuery(url: string): string {
return url.replace(directRequestRE, '$1').replace(trailingSeparatorRE, '')
}

export function injectQuery(url: string, queryToInject: string): string {
// encode percents for consistent behavior with pathToFileURL
Expand Down