From 97f8b4df3c9eb817ab2669e5c10b700802eec900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= Date: Wed, 30 Nov 2022 16:05:35 +0900 Subject: [PATCH] fix(importGlob): don't warn when CSS default import is not used (#11121) --- .../plugins/importGlob/fixture-c/a.css | 3 - .../plugins/importGlob/fixture-c/b.css | 3 - .../plugins/importGlob/fixture-c/c.module.css | 3 - .../plugins/importGlob/fixture.test.ts | 66 +---------- packages/vite/src/node/optimizer/scan.ts | 2 +- .../vite/src/node/plugins/importMetaGlob.ts | 107 ++++++++++++------ .../glob-import/__tests__/glob-import.spec.ts | 27 +++++ playground/glob-import/index.html | 4 + 8 files changed, 112 insertions(+), 103 deletions(-) delete mode 100644 packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/a.css delete mode 100644 packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/b.css delete mode 100644 packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/c.module.css diff --git a/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/a.css b/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/a.css deleted file mode 100644 index 5451a331f9cea4..00000000000000 --- a/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/a.css +++ /dev/null @@ -1,3 +0,0 @@ -.a { - color: red; -} diff --git a/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/b.css b/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/b.css deleted file mode 100644 index b28d1ab0817412..00000000000000 --- a/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/b.css +++ /dev/null @@ -1,3 +0,0 @@ -.b { - color: red; -} diff --git a/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/c.module.css b/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/c.module.css deleted file mode 100644 index b28d1ab0817412..00000000000000 --- a/packages/vite/src/node/__tests__/plugins/importGlob/fixture-c/c.module.css +++ /dev/null @@ -1,3 +0,0 @@ -.b { - color: red; -} diff --git a/packages/vite/src/node/__tests__/plugins/importGlob/fixture.test.ts b/packages/vite/src/node/__tests__/plugins/importGlob/fixture.test.ts index 4aef33bfc4fe6b..cd6dc7c6fc4deb 100644 --- a/packages/vite/src/node/__tests__/plugins/importGlob/fixture.test.ts +++ b/packages/vite/src/node/__tests__/plugins/importGlob/fixture.test.ts @@ -4,15 +4,12 @@ import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' import { transformGlobImport } from '../../../plugins/importMetaGlob' import { transformWithEsbuild } from '../../../plugins/esbuild' -import type { Logger } from '../../../logger' -import { createLogger } from '../../../logger' const __dirname = resolve(fileURLToPath(import.meta.url), '..') describe('fixture', async () => { const resolveId = (id: string) => id const root = resolve(__dirname, '..') - const logger = createLogger() it('transform', async () => { const id = resolve(__dirname, './fixture-a/index.ts') @@ -21,22 +18,14 @@ describe('fixture', async () => { ).code expect( - ( - await transformGlobImport(code, id, root, resolveId, logger) - )?.s.toString() + (await transformGlobImport(code, id, root, resolveId, true))?.s.toString() ).toMatchSnapshot() }) it('preserve line count', async () => { const getTransformedLineCount = async (code: string) => ( - await transformGlobImport( - code, - 'virtual:module', - root, - resolveId, - logger - ) + await transformGlobImport(code, 'virtual:module', root, resolveId, true) )?.s .toString() .split('\n').length @@ -61,13 +50,7 @@ describe('fixture', async () => { ].join('\n') expect( ( - await transformGlobImport( - code, - 'virtual:module', - root, - resolveId, - logger - ) + await transformGlobImport(code, 'virtual:module', root, resolveId, true) )?.s.toString() ).toMatchSnapshot() @@ -77,7 +60,7 @@ describe('fixture', async () => { 'virtual:module', root, resolveId, - logger + true ) expect('no error').toBe('should throw an error') } catch (err) { @@ -95,47 +78,8 @@ describe('fixture', async () => { expect( ( - await transformGlobImport(code, id, root, resolveId, logger, true) + await transformGlobImport(code, id, root, resolveId, true, true) )?.s.toString() ).toMatchSnapshot() }) - - it('warn when glob css without ?inline', async () => { - const logs: string[] = [] - const logger = { - warn(msg: string) { - logs.push(msg) - } - } as Logger - - await transformGlobImport( - "import.meta.glob('./fixture-c/*.css', { query: '?inline' })", - fileURLToPath(import.meta.url), - root, - resolveId, - logger - ) - expect(logs).toHaveLength(0) - - await transformGlobImport( - "import.meta.glob('./fixture-c/*.module.css')", - fileURLToPath(import.meta.url), - root, - resolveId, - logger - ) - expect(logs).toHaveLength(0) - - await transformGlobImport( - "import.meta.glob('./fixture-c/*.css')", - fileURLToPath(import.meta.url), - root, - resolveId, - logger - ) - expect(logs).toHaveLength(1) - expect(logs[0]).to.include( - 'Globbing CSS files without the ?inline query is deprecated' - ) - }) }) diff --git a/packages/vite/src/node/optimizer/scan.ts b/packages/vite/src/node/optimizer/scan.ts index d2c73cf5f1dadd..891be702cb6bb2 100644 --- a/packages/vite/src/node/optimizer/scan.ts +++ b/packages/vite/src/node/optimizer/scan.ts @@ -223,7 +223,7 @@ function esbuildScanPlugin( id, config.root, resolve, - config.logger + config.isProduction ) return result?.s.toString() || transpiledContents diff --git a/packages/vite/src/node/plugins/importMetaGlob.ts b/packages/vite/src/node/plugins/importMetaGlob.ts index 3f986f87ca49d7..82033ed3033dcd 100644 --- a/packages/vite/src/node/plugins/importMetaGlob.ts +++ b/packages/vite/src/node/plugins/importMetaGlob.ts @@ -1,6 +1,5 @@ import { isAbsolute, posix } from 'node:path' import micromatch from 'micromatch' -import colors from 'picocolors' import { stripLiteral } from 'strip-literal' import type { ArrayExpression, @@ -26,12 +25,10 @@ import type { ModuleNode } from '../server/moduleGraph' import type { ResolvedConfig } from '../config' import { evalValue, - generateCodeFrame, normalizePath, slash, transformStableResult } from '../utils' -import type { Logger } from '../logger' import { isCSSRequest, isModuleCSSRequest } from './css' const { isMatch, scan } = micromatch @@ -79,7 +76,7 @@ export function importGlobPlugin(config: ResolvedConfig): Plugin { id, config.root, (im) => this.resolve(im, id).then((i) => i?.id || im), - config.logger, + config.isProduction, config.experimental.importGlobRestoreExtension ) if (result) { @@ -320,6 +317,24 @@ const importPrefix = '__vite_glob_' const { basename, dirname, relative, join } = posix +const warnedCSSDefaultImportVarName = '__vite_warned_css_default_import' +const jsonStringifyInOneline = (input: any) => + JSON.stringify(input).replace(/[{,:]/g, '$& ').replace(/\}/g, ' }') +const createCssDefaultImportWarning = ( + globs: string[], + options: GeneralImportGlobOptions +) => + `if (!${warnedCSSDefaultImportVarName}) {` + + `${warnedCSSDefaultImportVarName} = true;` + + `console.warn(${JSON.stringify( + 'Default import of CSS without `?inline` is deprecated. ' + + "Add the `{ query: '?inline' }` glob option to fix this.\n" + + `For example: \`import.meta.glob(${jsonStringifyInOneline( + globs.length === 1 ? globs[0] : globs + )}, ${jsonStringifyInOneline({ ...options, query: '?inline' })})\`` + )});` + + `}` + export interface TransformGlobImportResult { s: MagicString matches: ParsedImportGlob[] @@ -334,7 +349,7 @@ export async function transformGlobImport( id: string, root: string, resolveId: IdResolver, - logger: Logger, + isProduction: boolean, restoreQueryExtension = false ): Promise { id = slash(id) @@ -365,7 +380,15 @@ export async function transformGlobImport( const staticImports = ( await Promise.all( matches.map( - async ({ globsResolved, isRelative, options, index, start, end }) => { + async ({ + globs, + globsResolved, + isRelative, + options, + index, + start, + end + }) => { const cwd = getCommonBase(globsResolved) ?? root const files = ( await fg(globsResolved, { @@ -391,25 +414,6 @@ export async function transformGlobImport( if (query && !query.startsWith('?')) query = `?${query}` - if ( - !query && // ignore custom queries - files.some( - (file) => isCSSRequest(file) && !isModuleCSSRequest(file) - ) - ) { - logger.warn( - `\n` + - colors.cyan(id) + - `\n` + - colors.reset(generateCodeFrame(code, start)) + - `\n` + - colors.yellow( - `Globbing CSS files without the ?inline query is deprecated. ` + - `Add the \`{ query: '?inline' }\` glob option to fix this.` - ) - ) - } - const resolvePaths = (file: string) => { if (!dir) { if (isRelative) @@ -434,6 +438,7 @@ export async function transformGlobImport( return { filePath, importPath } } + let includesCSS = false files.forEach((file, i) => { const paths = resolvePaths(file) const filePath = paths.filePath @@ -448,6 +453,10 @@ export async function transformGlobImport( importPath = `${importPath}${importQuery}` + const isCSS = + !query && isCSSRequest(file) && !isModuleCSSRequest(file) + includesCSS ||= isCSS + const importKey = options.import && options.import !== '*' ? options.import @@ -461,14 +470,36 @@ export async function transformGlobImport( staticImports.push( `import ${expression} from ${JSON.stringify(importPath)}` ) - objectProps.push(`${JSON.stringify(filePath)}: ${variableName}`) + if (!isProduction && isCSS) { + objectProps.push( + `get ${JSON.stringify( + filePath + )}() { ${createCssDefaultImportWarning( + globs, + options + )} return ${variableName} }` + ) + } else { + objectProps.push(`${JSON.stringify(filePath)}: ${variableName}`) + } } else { let importStatement = `import(${JSON.stringify(importPath)})` if (importKey) importStatement += `.then(m => m[${JSON.stringify(importKey)}])` - objectProps.push( - `${JSON.stringify(filePath)}: () => ${importStatement}` - ) + if (!isProduction && isCSS) { + objectProps.push( + `${JSON.stringify( + filePath + )}: () => { ${createCssDefaultImportWarning( + globs, + options + )} return ${importStatement}}` + ) + } else { + objectProps.push( + `${JSON.stringify(filePath)}: () => ${importStatement}` + ) + } } }) @@ -480,9 +511,21 @@ export async function transformGlobImport( originalLineBreakCount > 0 ? '\n'.repeat(originalLineBreakCount) : '' - const replacement = `/* #__PURE__ */ Object.assign({${objectProps.join( - ',' - )}${lineBreaks}})` + + let replacement: string + if (!isProduction && includesCSS) { + replacement = + '/* #__PURE__ */ Object.assign(' + + '(() => {' + + `let ${warnedCSSDefaultImportVarName} = false;` + + `return {${objectProps.join(',')}${lineBreaks}};` + + '})()' + + ')' + } else { + replacement = `/* #__PURE__ */ Object.assign({${objectProps.join( + ',' + )}${lineBreaks}})` + } s.overwrite(start, end, replacement) return staticImports diff --git a/playground/glob-import/__tests__/glob-import.spec.ts b/playground/glob-import/__tests__/glob-import.spec.ts index c9a4c4ffe34d5e..9a10b2b7bf6892 100644 --- a/playground/glob-import/__tests__/glob-import.spec.ts +++ b/playground/glob-import/__tests__/glob-import.spec.ts @@ -7,8 +7,11 @@ import { findAssetFile, getColor, isBuild, + isServe, page, removeFile, + untilBrowserLogAfter, + viteTestUrl, withRetry } from '~utils' @@ -190,6 +193,30 @@ test('tree-shake eager css', async () => { } }) +test('warn CSS default import', async () => { + const logs = await untilBrowserLogAfter( + () => page.goto(viteTestUrl), + 'Ran scripts' + ) + const noTreeshakeCSSMessage = + 'For example: `import.meta.glob("/no-tree-shake.css", { "eager": true, "query": "?inline" })`' + const treeshakeCSSMessage = + 'For example: `import.meta.glob("/tree-shake.css", { "eager": true, "query": "?inline" })`' + + expect( + logs.some((log) => log.includes(noTreeshakeCSSMessage)), + `expected logs to include a message including ${JSON.stringify( + noTreeshakeCSSMessage + )}` + ).toBe(isServe) + expect( + logs.every((log) => !log.includes(treeshakeCSSMessage)), + `expected logs not to include a message including ${JSON.stringify( + treeshakeCSSMessage + )}` + ).toBe(true) +}) + test('escapes special chars in globs without mangling user supplied glob suffix', async () => { // the escape dir contains subdirectories where each has a name that needs escaping for glob safety // inside each of them is a glob.js that exports the result of a relative glob `./**/*.js` diff --git a/playground/glob-import/index.html b/playground/glob-import/index.html index 359b4fb75ef5f8..5c07bd11690281 100644 --- a/playground/glob-import/index.html +++ b/playground/glob-import/index.html @@ -140,3 +140,7 @@

Escape alias glob

.map(([glob]) => glob) document.querySelector('.escape-alias').textContent = alias.sort().join('\n') + +