From 047ac6ff8de0c68a4a99b68d0c974a29e3223294 Mon Sep 17 00:00:00 2001
From: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
Date: Tue, 20 May 2025 18:56:20 +0900
Subject: [PATCH 1/3] fix(css): should not wrap with double quote when the url
rebase feature bailed out
---
packages/vite/src/node/plugins/css.ts | 4 ++++
playground/css/__tests__/sass-tests.ts | 6 ++++++
playground/css/index.html | 3 +++
playground/css/nested/_index.scss | 7 +++++++
4 files changed, 20 insertions(+)
diff --git a/packages/vite/src/node/plugins/css.ts b/packages/vite/src/node/plugins/css.ts
index a0f386fd9bd135..5fd8ee1746425e 100644
--- a/packages/vite/src/node/plugins/css.ts
+++ b/packages/vite/src/node/plugins/css.ts
@@ -2058,6 +2058,10 @@ async function doUrlReplace(
}
let newUrl = await replacer(rawUrl)
+ if (rawUrl === newUrl) {
+ return matched
+ }
+
// The new url might need wrapping even if the original did not have it, e.g. if a space was added during replacement
if (wrap === '' && newUrl !== encodeURI(newUrl)) {
wrap = '"'
diff --git a/playground/css/__tests__/sass-tests.ts b/playground/css/__tests__/sass-tests.ts
index c9a3c9a5d42d07..e6ccdb2c8d7e25 100644
--- a/playground/css/__tests__/sass-tests.ts
+++ b/playground/css/__tests__/sass-tests.ts
@@ -15,6 +15,9 @@ export const sassTest = () => {
const atImport = await page.$('.sass-at-import')
const atImportAlias = await page.$('.sass-at-import-alias')
const urlStartsWithVariable = await page.$('.sass-url-starts-with-variable')
+ const urlStartsWithVariableConcat = await page.$(
+ '.sass-url-starts-with-variable-concat',
+ )
const urlStartsWithFunctionCall = await page.$(
'.sass-url-starts-with-function-call',
)
@@ -32,6 +35,9 @@ export const sassTest = () => {
expect(await getBg(urlStartsWithVariable)).toMatch(
isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
)
+ expect(await getBg(urlStartsWithVariableConcat)).toMatch(
+ isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
+ )
expect(await getBg(urlStartsWithFunctionCall)).toMatch(
isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
)
diff --git a/playground/css/index.html b/playground/css/index.html
index 17e680d349a3dc..6d96929d7f8300 100644
--- a/playground/css/index.html
+++ b/playground/css/index.html
@@ -35,6 +35,9 @@
CSS
@import from SASS _partial: This should be orchid
url starts with variable
+
+ url starts with variable and contains concat
+
url starts with function call
diff --git a/playground/css/nested/_index.scss b/playground/css/nested/_index.scss
index c0767a3f4431c6..878b97fd957197 100644
--- a/playground/css/nested/_index.scss
+++ b/playground/css/nested/_index.scss
@@ -20,6 +20,13 @@ $var: '/ok.png';
background-position: center;
}
+$var-c1: '/ok';
+$var-c2: '.png';
+.sass-url-starts-with-variable-concat {
+ background: url($var-c1 + $var-c2);
+ background-position: center;
+}
+
$var2: '/OK.PNG';
.sass-url-starts-with-function-call {
background: url(string.to-lower-case($var2));
From 34d1719c7a859e1dd30006609f62e8ca3655dc64 Mon Sep 17 00:00:00 2001
From: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
Date: Wed, 21 May 2025 13:04:35 +0900
Subject: [PATCH 2/3] chore: add comment
---
packages/vite/src/node/plugins/css.ts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/packages/vite/src/node/plugins/css.ts b/packages/vite/src/node/plugins/css.ts
index 5fd8ee1746425e..63e71a6f2f3486 100644
--- a/packages/vite/src/node/plugins/css.ts
+++ b/packages/vite/src/node/plugins/css.ts
@@ -2058,6 +2058,8 @@ async function doUrlReplace(
}
let newUrl = await replacer(rawUrl)
+ // If the replacer output is same with the input, we don't need to wrap it with quotes.
+ // We also want to keep it as-is because the replacer returns the url as-is when the input starts with a variable.
if (rawUrl === newUrl) {
return matched
}
From 360584f8345c3890fdaef5687ca56add0826db17 Mon Sep 17 00:00:00 2001
From: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
Date: Fri, 23 May 2025 16:29:37 +0900
Subject: [PATCH 3/3] fix: bail out replacement properly
---
packages/vite/src/node/plugins/css.ts | 96 ++++++++++++++++++--------
playground/css/__tests__/sass-tests.ts | 12 ++++
playground/css/__tests__/tests.ts | 6 ++
playground/css/index.html | 9 +++
playground/css/nested/_index.scss | 10 +++
playground/css/nested/nested.less | 5 ++
6 files changed, 109 insertions(+), 29 deletions(-)
diff --git a/packages/vite/src/node/plugins/css.ts b/packages/vite/src/node/plugins/css.ts
index 629fb166a5ac31..5ba7c8b055e457 100644
--- a/packages/vite/src/node/plugins/css.ts
+++ b/packages/vite/src/node/plugins/css.ts
@@ -1901,10 +1901,15 @@ type CssUrlResolver = (
) =>
| [url: string, id: string | undefined]
| Promise<[url: string, id: string | undefined]>
+/**
+ * replace URL references
+ *
+ * When returning `false`, it keeps the content as-is
+ */
type CssUrlReplacer = (
- url: string,
- importer?: string,
-) => string | Promise
+ unquotedUrl: string,
+ rawUrl: string,
+) => string | false | Promise
// https://drafts.csswg.org/css-syntax-3/#identifier-code-point
export const cssUrlRE =
/(? {
+ const isQuoted = rawUrl[0] === '"' || rawUrl[0] === "'"
+ // matches `url($foo)`
+ if (!isQuoted && unquotedUrl[0] === '$') {
+ return true
+ }
+ // matches `url(#{foo})` and `url('#{foo}')`
+ return unquotedUrl.startsWith('#{')
+ }
+
const internalLoad = async (file: string, rootFile: string) => {
const result = await rebaseUrls(
environment,
file,
rootFile,
alias,
- '$',
resolvers.sass,
+ skipRebaseUrls,
)
if (result.contents) {
return result.contents
@@ -2535,6 +2554,16 @@ const makeModernCompilerScssWorker = (
sassOptions.url = pathToFileURL(options.filename)
sassOptions.sourceMap = options.enableSourcemap
+ const skipRebaseUrls = (unquotedUrl: string, rawUrl: string) => {
+ const isQuoted = rawUrl[0] === '"' || rawUrl[0] === "'"
+ // matches `url($foo)`
+ if (!isQuoted && unquotedUrl[0] === '$') {
+ return true
+ }
+ // matches `url(#{foo})` and `url('#{foo}')`
+ return unquotedUrl.startsWith('#{')
+ }
+
const internalImporter: Sass.Importer<'async'> = {
async canonicalize(url, context) {
const importer = context.containingUrl
@@ -2568,8 +2597,8 @@ const makeModernCompilerScssWorker = (
fileURLToPath(canonicalUrl),
options.filename,
alias,
- '$',
resolvers.sass,
+ skipRebaseUrls,
)
const contents =
result.contents ?? (await fsp.readFile(result.file, 'utf-8'))
@@ -2715,8 +2744,8 @@ async function rebaseUrls(
file: string,
rootFile: string,
alias: Alias[],
- variablePrefix: string,
resolver: ResolveIdFn,
+ ignoreUrl?: (unquotedUrl: string, rawUrl: string) => boolean,
): Promise<{ file: string; contents?: string }> {
file = path.resolve(file) // ensure os-specific flashes
// in the same dir, no need to rebase
@@ -2739,20 +2768,22 @@ async function rebaseUrls(
}
let rebased
- const rebaseFn = async (url: string) => {
- if (url[0] === '/') return url
- // ignore url's starting with variable
- if (url.startsWith(variablePrefix)) return url
+ const rebaseFn = async (unquotedUrl: string, rawUrl: string) => {
+ if (ignoreUrl?.(unquotedUrl, rawUrl)) return false
+ if (unquotedUrl[0] === '/') return unquotedUrl
// match alias, no need to rewrite
for (const { find } of alias) {
const matches =
- typeof find === 'string' ? url.startsWith(find) : find.test(url)
+ typeof find === 'string'
+ ? unquotedUrl.startsWith(find)
+ : find.test(unquotedUrl)
if (matches) {
- return url
+ return unquotedUrl
}
}
const absolute =
- (await resolver(environment, url, file)) || path.resolve(fileDir, url)
+ (await resolver(environment, unquotedUrl, file)) ||
+ path.resolve(fileDir, unquotedUrl)
const relative = path.relative(rootDir, absolute)
return normalizePath(relative)
}
@@ -2784,6 +2815,13 @@ const makeLessWorker = (
alias: Alias[],
maxWorkers: number | undefined,
) => {
+ const skipRebaseUrls = (unquotedUrl: string, _rawUrl: string) => {
+ // matches both
+ // - interpolation: `url('@{foo}')`
+ // - variable: `url(@foo)`
+ return unquotedUrl[0] === '@'
+ }
+
const viteLessResolve = async (
filename: string,
dir: string,
@@ -2808,8 +2846,8 @@ const makeLessWorker = (
resolved,
rootFile,
alias,
- '@',
resolvers.less,
+ skipRebaseUrls,
)
return {
resolved,
diff --git a/playground/css/__tests__/sass-tests.ts b/playground/css/__tests__/sass-tests.ts
index e6ccdb2c8d7e25..1c0dcf7b90b17f 100644
--- a/playground/css/__tests__/sass-tests.ts
+++ b/playground/css/__tests__/sass-tests.ts
@@ -15,6 +15,12 @@ export const sassTest = () => {
const atImport = await page.$('.sass-at-import')
const atImportAlias = await page.$('.sass-at-import-alias')
const urlStartsWithVariable = await page.$('.sass-url-starts-with-variable')
+ const urlStartsWithVariableInterpolation1 = await page.$(
+ '.sass-url-starts-with-interpolation1',
+ )
+ const urlStartsWithVariableInterpolation2 = await page.$(
+ '.sass-url-starts-with-interpolation2',
+ )
const urlStartsWithVariableConcat = await page.$(
'.sass-url-starts-with-variable-concat',
)
@@ -35,6 +41,12 @@ export const sassTest = () => {
expect(await getBg(urlStartsWithVariable)).toMatch(
isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
)
+ expect(await getBg(urlStartsWithVariableInterpolation1)).toMatch(
+ isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
+ )
+ expect(await getBg(urlStartsWithVariableInterpolation2)).toMatch(
+ isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
+ )
expect(await getBg(urlStartsWithVariableConcat)).toMatch(
isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
)
diff --git a/playground/css/__tests__/tests.ts b/playground/css/__tests__/tests.ts
index 086180b9a64b44..bab808c06f378d 100644
--- a/playground/css/__tests__/tests.ts
+++ b/playground/css/__tests__/tests.ts
@@ -98,6 +98,9 @@ export const tests = (isLightningCSS: boolean) => {
const atImportAlias = await page.$('.less-at-import-alias')
const atImportUrlOmmer = await page.$('.less-at-import-url-ommer')
const urlStartsWithVariable = await page.$('.less-url-starts-with-variable')
+ const urlStartsWithInterpolation = await page.$(
+ '.less-url-starts-with-interpolation',
+ )
expect(await getColor(imported)).toBe('blue')
expect(await getColor(atImport)).toBe('darkslateblue')
@@ -112,6 +115,9 @@ export const tests = (isLightningCSS: boolean) => {
expect(await getBg(urlStartsWithVariable)).toMatch(
isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
)
+ expect(await getBg(urlStartsWithInterpolation)).toMatch(
+ isBuild ? /ok-[-\w]+\.png/ : `${viteTestUrl}/ok.png`,
+ )
if (isBuild) return
diff --git a/playground/css/index.html b/playground/css/index.html
index 6d96929d7f8300..cc30e89693d920 100644
--- a/playground/css/index.html
+++ b/playground/css/index.html
@@ -35,6 +35,12 @@ CSS
@import from SASS _partial: This should be orchid
url starts with variable
+
+ url starts with interpolation 1
+
+
+ url starts with interpolation 2
+
url starts with variable and contains concat
@@ -65,6 +71,9 @@ CSS
@import url() from Less: This should be darkorange
url starts with variable
+
+ url starts with interpolation
+
tests Less's `data-uri()` function with relative image paths
diff --git a/playground/css/nested/_index.scss b/playground/css/nested/_index.scss
index 878b97fd957197..193828696a1004 100644
--- a/playground/css/nested/_index.scss
+++ b/playground/css/nested/_index.scss
@@ -20,6 +20,16 @@ $var: '/ok.png';
background-position: center;
}
+.sass-url-starts-with-interpolation1 {
+ background: url(#{$var});
+ background-position: center;
+}
+
+.sass-url-starts-with-interpolation2 {
+ background: url('#{$var}');
+ background-position: center;
+}
+
$var-c1: '/ok';
$var-c2: '.png';
.sass-url-starts-with-variable-concat {
diff --git a/playground/css/nested/nested.less b/playground/css/nested/nested.less
index 25aa1944d32c14..ecd1b9bff4203a 100644
--- a/playground/css/nested/nested.less
+++ b/playground/css/nested/nested.less
@@ -10,6 +10,11 @@
@var: '/ok.png';
.less-url-starts-with-variable {
+ background: url(@var);
+ background-position: center;
+}
+
+.less-url-starts-with-interpolation {
background: url('@{var}');
background-position: center;
}