From 6aea6c066d24a7cf51eb5678ce9c0fb9f5349cf9 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Sat, 23 Apr 2022 00:51:21 +0900 Subject: [PATCH 1/2] fix(css): hoist external @import for non-split css --- 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/main.js | 2 +- packages/playground/lib/src/main2.js | 5 ++++ .../playground/lib/vite.dyimport.config.js | 1 - packages/vite/src/node/plugins/css.ts | 28 +++++++++++-------- 8 files changed, 50 insertions(+), 14 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/main.js b/packages/playground/lib/src/main.js index cb2fb3b842dc4f..a4f5f4e4f11f15 100644 --- a/packages/playground/lib/src/main.js +++ b/packages/playground/lib/src/main.js @@ -1,4 +1,4 @@ -export default function myLib(sel) { +export default async function myLib(sel) { // Force esbuild spread helpers (https://github.com/evanw/esbuild/issues/951) console.log({ ...'foo' }) 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[] From 063c6dbb307202c59879fa7f8a026f42bd4ac55c Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Thu, 5 May 2022 17:19:43 +0900 Subject: [PATCH 2/2] test: remove unnecessary change --- packages/playground/lib/src/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playground/lib/src/main.js b/packages/playground/lib/src/main.js index a4f5f4e4f11f15..cb2fb3b842dc4f 100644 --- a/packages/playground/lib/src/main.js +++ b/packages/playground/lib/src/main.js @@ -1,4 +1,4 @@ -export default async function myLib(sel) { +export default function myLib(sel) { // Force esbuild spread helpers (https://github.com/evanw/esbuild/issues/951) console.log({ ...'foo' })