Skip to content

Commit

Permalink
fix(importGlob): don't warn when CSS default import is not used (#11121)
Browse files Browse the repository at this point in the history
  • Loading branch information
sapphi-red authored Nov 30, 2022
1 parent 1db52bf commit 97f8b4d
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 103 deletions.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand All @@ -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()

Expand All @@ -77,7 +60,7 @@ describe('fixture', async () => {
'virtual:module',
root,
resolveId,
logger
true
)
expect('no error').toBe('should throw an error')
} catch (err) {
Expand All @@ -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'
)
})
})
2 changes: 1 addition & 1 deletion packages/vite/src/node/optimizer/scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ function esbuildScanPlugin(
id,
config.root,
resolve,
config.logger
config.isProduction
)

return result?.s.toString() || transpiledContents
Expand Down
107 changes: 75 additions & 32 deletions packages/vite/src/node/plugins/importMetaGlob.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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[]
Expand All @@ -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)
Expand Down Expand Up @@ -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, {
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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}`
)
}
}
})

Expand All @@ -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
Expand Down
27 changes: 27 additions & 0 deletions playground/glob-import/__tests__/glob-import.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ import {
findAssetFile,
getColor,
isBuild,
isServe,
page,
removeFile,
untilBrowserLogAfter,
viteTestUrl,
withRetry
} from '~utils'

Expand Down Expand Up @@ -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`
Expand Down
4 changes: 4 additions & 0 deletions playground/glob-import/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>

0 comments on commit 97f8b4d

Please sign in to comment.