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): hoist charset #7678

Merged
merged 1 commit into from Apr 11, 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
9 changes: 9 additions & 0 deletions packages/playground/css/__tests__/css.spec.ts
Expand Up @@ -247,6 +247,15 @@ test('inline css modules', async () => {
expect(css).toMatch(/\.inline-module__apply-color-inline___[\w-]{5}/)
})

if (isBuild) {
test('@charset hoist', async () => {
serverLogs.forEach((log) => {
// no warning from esbuild css minifier
expect(log).not.toMatch('"@charset" must be the first rule in the file')
})
})
}

test('@import dependency w/ style entry', async () => {
expect(await getColor('.css-dep')).toBe('purple')
})
Expand Down
5 changes: 5 additions & 0 deletions packages/playground/css/charset.css
@@ -0,0 +1,5 @@
@charset "utf-8";

.utf8 {
color: green;
}
3 changes: 3 additions & 0 deletions packages/playground/css/index.html
Expand Up @@ -96,6 +96,9 @@ <h1>CSS</h1>
<p>Inline CSS module:</p>
<pre class="modules-inline"></pre>

<p>CSS with @charset:</p>
<pre class="charset-css"></pre>

<p class="css-dep">
@import dependency w/ style enrtrypoints: this should be purple
</p>
Expand Down
3 changes: 3 additions & 0 deletions packages/playground/css/main.js
Expand Up @@ -41,6 +41,9 @@ text(
import inlineMod from './inline.module.css?inline'
text('.modules-inline', inlineMod)

import charset from './charset.css'
text('.charset-css', charset)

import './dep.css'
import './glob-dep.css'

Expand Down
24 changes: 15 additions & 9 deletions packages/vite/src/node/plugins/css.ts
Expand Up @@ -427,10 +427,10 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
return `./${path.posix.basename(filename)}`
}
})
// only external @imports should exist at this point - and they need to
// be hoisted to the top of the CSS chunk per spec (#1845)
if (css.includes('@import')) {
css = await hoistAtImports(css)
// only external @imports and @charset should exist at this point
// hoist them to the top of the CSS chunk per spec (#1845 and #6333)
if (css.includes('@import') || css.includes('@charset')) {
css = await hoistAtRules(css)
}
if (minify && config.build.minify) {
css = await minifyCSS(css, config)
Expand Down Expand Up @@ -1109,27 +1109,33 @@ async function minifyCSS(css: string, config: ResolvedConfig) {
// #1845
// CSS @import can only appear at top of the file. We need to hoist all @import
// to top when multiple files are concatenated.
async function hoistAtImports(css: string) {
// #6333
// CSS @charset must be the top-first in the file, hoist to top too
async function hoistAtRules(css: string) {
const postcss = await import('postcss')
return (await postcss.default([AtImportHoistPlugin]).process(css)).css
return (await postcss.default([AtRuleHoistPlugin]).process(css)).css
}

const AtImportHoistPlugin: PostCSS.PluginCreator<any> = () => {
const AtRuleHoistPlugin: PostCSS.PluginCreator<any> = () => {
return {
postcssPlugin: 'vite-hoist-at-imports',
postcssPlugin: 'vite-hoist-at-rules',
Once(root) {
const imports: PostCSS.AtRule[] = []
let charset: PostCSS.AtRule | undefined
root.walkAtRules((rule) => {
if (rule.name === 'import') {
// record in reverse so that can simply prepend to preserve order
imports.unshift(rule)
} else if (!charset && rule.name === 'charset') {
charset = rule
}
})
imports.forEach((i) => root.prepend(i))
if (charset) root.prepend(charset)
}
}
}
AtImportHoistPlugin.postcss = true
AtRuleHoistPlugin.postcss = true

// Preprocessor support. This logic is largely replicated from @vue/compiler-sfc

Expand Down