-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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: url() with variable or relative path in sass/scss is broken (fixes #7651) #8765
Conversation
✅ Deploy Preview for vite-docs-main canceled.
|
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.
Thank you for the PR. Could you add a test for this?
@@ -1478,7 +1488,7 @@ async function rebaseUrls( | |||
} | |||
|
|||
if (hasUrls) { | |||
rebased = await rewriteCssUrls(rebased || content, rebaseFn) | |||
rebased = await rewriteCssUrls(rebased || content, rebaseFn, file) |
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 could pass variablePrefix
($
, @
) from rebaseUrls
is called instead of file name? It will remove many conditions.
(inLess && rawUrl.startsWith('@')) || | ||
((inSass || inScss) && rawUrl.startsWith('$')) | ||
) { | ||
return `url('${rawUrl}')` |
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.
return `url('${rawUrl}')` | |
return `url(${rawUrl})` |
I think rawUrl
include quotes if needed.
Any news on this PR? |
Is there anything I can do to help get this PR merged? I'd love to migrate our projects over from Webpack to Vite, but this is a real blocker for us. |
We need tests for this. Feel free to send another PR that supersedes this with tests! |
Any news? |
I tried implementing this into a new PR with tests but a load of other seemingly unrelated tests started failing because of it. This is out of my area of expertise so not something I'm going to be able to help with unfortunately. |
Description
fixes #7651
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).