From 5280908972219392a0d8daa2a33553868ca7072b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= Date: Fri, 6 May 2022 18:26:47 +0900 Subject: [PATCH] fix(css): hoist external @import for non-split css (#8022) --- packages/playground/lib/__tests__/lib.spec.ts | 7 +++++ packages/playground/lib/__tests__/serve.js | 14 +++++++++- packages/playground/lib/src/dynamic.css | 4 +++ packages/playground/lib/src/index.css | 3 ++ packages/playground/lib/src/main2.js | 5 ++++ .../playground/lib/vite.dyimport.config.js | 1 - packages/vite/src/node/plugins/css.ts | 28 +++++++++++-------- 7 files changed, 49 insertions(+), 13 deletions(-) create mode 100644 packages/playground/lib/src/dynamic.css create mode 100644 packages/playground/lib/src/index.css diff --git a/packages/playground/lib/__tests__/lib.spec.ts b/packages/playground/lib/__tests__/lib.spec.ts index f1e93e90d8357b..cc5887c2777fcb 100644 --- a/packages/playground/lib/__tests__/lib.spec.ts +++ b/packages/playground/lib/__tests__/lib.spec.ts @@ -35,6 +35,13 @@ if (isBuild) { ) expect(code).not.toMatch('__vitePreload') }) + + test('@import hoist', async () => { + serverLogs.forEach((log) => { + // no warning from esbuild css minifier + expect(log).not.toMatch('All "@import" rules must come first') + }) + }) } else { test('dev', async () => { expect(await page.textContent('.demo')).toBe('It works') diff --git a/packages/playground/lib/__tests__/serve.js b/packages/playground/lib/__tests__/serve.js index eac6980286af52..2534545de5c221 100644 --- a/packages/playground/lib/__tests__/serve.js +++ b/packages/playground/lib/__tests__/serve.js @@ -9,6 +9,8 @@ const { ports } = require('../../testUtils') const port = (exports.port = ports.lib) +global.serverLogs = [] + /** * @param {string} root * @param {boolean} isBuildTest @@ -16,6 +18,8 @@ const port = (exports.port = ports.lib) exports.serve = async function serve(root, isBuildTest) { // build first + setupConsoleWarnCollector() + if (!isBuildTest) { const { createServer } = require('vite') process.env.VITE_INLINE = 'inline-serve' @@ -55,7 +59,7 @@ exports.serve = async function serve(root, isBuildTest) { await build({ root, - logLevel: 'silent', + logLevel: 'warn', // output esbuild warns configFile: path.resolve(__dirname, '../vite.dyimport.config.js') }) @@ -89,3 +93,11 @@ exports.serve = async function serve(root, isBuildTest) { }) } } + +function setupConsoleWarnCollector() { + const warn = console.warn + console.warn = (...args) => { + global.serverLogs.push(args.join(' ')) + return warn.call(console, ...args) + } +} diff --git a/packages/playground/lib/src/dynamic.css b/packages/playground/lib/src/dynamic.css new file mode 100644 index 00000000000000..4378c8d328cfbe --- /dev/null +++ b/packages/playground/lib/src/dynamic.css @@ -0,0 +1,4 @@ +@import 'https://cdn.jsdelivr.net/npm/@mdi/font@5.9.55/css/materialdesignicons.min.css'; +.dynamic { + color: red; +} diff --git a/packages/playground/lib/src/index.css b/packages/playground/lib/src/index.css new file mode 100644 index 00000000000000..b0bd670bd9ecad --- /dev/null +++ b/packages/playground/lib/src/index.css @@ -0,0 +1,3 @@ +.index { + color: blue; +} diff --git a/packages/playground/lib/src/main2.js b/packages/playground/lib/src/main2.js index 0c729fad8d165c..f19a16bb128949 100644 --- a/packages/playground/lib/src/main2.js +++ b/packages/playground/lib/src/main2.js @@ -1,4 +1,9 @@ +import './index.css' + export default async function message(sel) { const message = await import('./message.js') + + await import('./dynamic.css') + document.querySelector(sel).textContent = message.default } diff --git a/packages/playground/lib/vite.dyimport.config.js b/packages/playground/lib/vite.dyimport.config.js index 76311f4b8ba138..d738503f0c9d09 100644 --- a/packages/playground/lib/vite.dyimport.config.js +++ b/packages/playground/lib/vite.dyimport.config.js @@ -6,7 +6,6 @@ const path = require('path') */ module.exports = { build: { - minify: false, lib: { entry: path.resolve(__dirname, 'src/main2.js'), formats: ['es'], diff --git a/packages/vite/src/node/plugins/css.ts b/packages/vite/src/node/plugins/css.ts index cd57acd1690902..c61e059867080b 100644 --- a/packages/vite/src/node/plugins/css.ts +++ b/packages/vite/src/node/plugins/css.ts @@ -443,13 +443,7 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin { } }) // 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) - } + css = await finalizeCss(css, minify, config) return css } @@ -559,10 +553,7 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin { let extractedCss = outputToExtractedCSSMap.get(opts) if (extractedCss && !hasEmitted) { hasEmitted = true - // minify css - if (config.build.minify) { - extractedCss = await minifyCSS(extractedCss, config) - } + extractedCss = await finalizeCss(extractedCss, true, config) this.emitFile({ name: 'style.css', type: 'asset', @@ -922,6 +913,21 @@ function combineSourcemapsIfExists( : map1 } +async function finalizeCss( + css: string, + minify: boolean, + config: ResolvedConfig +) { + // hoist external @imports and @charset 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) + } + return css +} + interface PostCSSConfigResult { options: PostCSS.ProcessOptions plugins: PostCSS.Plugin[]