fix: url() with variable or relative path in sass/scss is broken (fixes #7651)#8765
fix: url() with variable or relative path in sass/scss is broken (fixes #7651)#8765frankyeyq wants to merge 2 commits intovitejs:mainfrom
Conversation
✅ Deploy Preview for vite-docs-main canceled.
|
sapphi-red
left a comment
There was a problem hiding this comment.
Thank you for the PR. Could you add a test for this?
|
|
||
| if (hasUrls) { | ||
| rebased = await rewriteCssUrls(rebased || content, rebaseFn) | ||
| rebased = await rewriteCssUrls(rebased || content, rebaseFn, file) |
There was a problem hiding this comment.
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.
| 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).