-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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(build-import-analysis): should not append ?used when css request has ?url or ?raw #11910
Conversation
Can you help review this PR @sapphi-red @bluwy Thanks. |
I think adding
|
…has ?url or ?raw (fix vitejs#11893)
9759fc2
to
bdf93fd
Compare
done |
@@ -361,7 +361,7 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin { | |||
// because there is no way to check whether the default export is used | |||
(source.slice(expStart, start).includes('from') || isDynamicImport) && | |||
// already has ?used query (by import.meta.glob) | |||
!specifier.match(/\?used(&|$)/) && | |||
!specifier.match(/\?(used|raw|url)(&|$)/) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use SPECIAL_QUERY_RE
to check the special queries, and leave !specifier.match(/\?used(&|$)/)
as is.
PS: I also tried to implement the idea I mentioned in discord last week. It's tricky since dynamic imports of css makes it harder to process it, which reminds me that I think we haven't logged the warnings for dynamic imports of CSS yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What warnings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have some warnings at
vite/packages/vite/src/node/plugins/importAnalysis.ts
Lines 527 to 531 in a1019f8
colors.yellow( | |
`Default and named imports from CSS files are deprecated. ` + | |
`Use the ?inline query instead. ` + | |
`For example: ${newImport}`, | |
), |
vite/packages/vite/src/node/plugins/importMetaGlob.ts
Lines 330 to 334 in a1019f8
'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' })})\``, |
?used
hack. I don't think it's related to this PR now though, and we can go in with this quick fix for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Done with SPECIAL_QUERY_RE
Speaking of dynamic import CSS, is there an alternative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speaking of dynamic import CSS, is there an alternative?
import('./foo.css').then(mod => mod.default)
should now be replaced with import('./foo.css?inline').then(mod => mod.default)
. Dynamic import without using default import (import('./foo.css')
) is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Description
fix #11893
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).