Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

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 () => {
sapphi-red marked this conversation as resolved.
Show resolved Hide resolved
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>