-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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: support importing css with ?url #5940
Conversation
@Niputi I don't believe this should be labelled as a feature. Currently when you import a CSS file using
|
we have been using the |
@Niputi is right, this targets |
@@ -161,7 +162,7 @@ export function cssPlugin(config: ResolvedConfig): Plugin { | |||
}, | |||
|
|||
async transform(raw, id) { | |||
if (!isCSSRequest(id) || commonjsProxyRE.test(id)) { | |||
if (!isCSSRequest(id) || commonjsProxyRE.test(id) || isURLRequest(id)) { |
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.
Maybe we check using SPECIAL_QUERY_RE instead?
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.
That would capture things other than ?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 the four queries (worker, sharedworker, raw, url) captured by the regex should all be skipped for the css plugin. The former two are handled by the workerPlugin
, the latter two are by the assetPlugin
. Pretty sure skipping raw would be fine here too, there's not much reason to transform a raw import. It might indirectly fix #5724.
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.
Though if we want to scope the fix to ?url only, I'm fine with the current change too.
Evan's comment here might be relevant as well, and I'm not sure if there's more needed to address that. |
Sounds like #5796 is going to cover this, so this pr is not needed. |
Description
This fixes importing a URL of a CSS file:
Additional context
The
vite:css
plugin should be checking for query params when doing its transforms. This makes it do so.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).