fix(importGlob): don't warn when CSS default import is not used (#11121)

vitejs · Nov 30, 2022 · 97f8b4d · 97f8b4d
1 parent 1db52bf
commit 97f8b4d
Show file tree

Hide file tree

Showing 8 changed files with 112 additions and 103 deletions.
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
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
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
diff --git 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
@@ -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
@@ -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<TransformGlobImportResult | null> {
   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
@@ -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
@@ -140,3 +140,7 @@ <h2>Escape alias glob</h2>
     .map(([glob]) => glob)
   document.querySelector('.escape-alias').textContent = alias.sort().join('\n')
 </script>
+
+<script type="module">
+  console.log('Ran scripts')
+</script>