fix(css): var in image-set

[Bigfish8]

Description

fix #7874

Additional context

the case in #7874

input:

background-image: image-set(var(--hero--image-webp) 1x, var(--hero--image-jpg) 1x);

output:

background-image: image-set(url(var(--hero--image-webp) ) 1x, var(--hero--image-jpg) 1x);

step:

1. cssImageSetRE match the image-set(var(--hero--image-webp) (note: there only one ))
2. replace var(--hero--image-webp to url(var(--hero--image-webp)

it means that if the css is:
.some-class { --some-var: './a.png'; background-image: -webkit-image-set(var(--some-var) 1x, './b.png' 2x)
The cssImageSetRE will only match image-set(var(--some-var) and do not handle './b.png'.

But I think this is OK, because of the demo metioned in #7874. Url like './b.png' in image-set is invalid, only url('./b.png') is valid.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[patak-dev]

I think it would be better to do a review of the current flow. In MDN, there is an example without using url(), https://developer.mozilla.org/en-US/docs/Web/CSS/image/image-set

  background-image: image-set(
    "large-balloons.avif" type("image/avif"),
    "large-balloons.jpg" type("image/jpeg"));

Maybe the regex could be extended like: (?<=image-set\()(?:var\([^\)]*\)|[^)])*(?=\)) (and also including url(), type(), etc. And the order in https://github.com/vitejs/vite/pull/7921/files#diff-2cfbd4f4d8c32727cd8e1a561cffbde0b384a3ce0789340440e144f9d64c10f6R993 could be reversed:

const rewriterToUse = isCssUrl ? rewriteCssUrls : rewriteCssImageSet

to

const rewriterToUse = isCssUrl ? rewriteCssImageSet : rewriteCssUrls

So we stop processing image sets that have url() inside with the rewriteCssUrls util, and instead augment rewriteCssImageSet to properly handle url() when used.

[patak-dev]

Should we split after ), ', " here? What happens if we have something like var( --my-var)?

[Bigfish8]

Thanks for review!
Yes, the space inside var() or url() will cause some trouble.Maybe I can clean space inside it.

[patak-dev]

Thank you for pushing forward with a fix! ;)
Maybe a regex is better to match the first part? var(), url(), '...', "..." ?

[Bigfish8]

Update.
According to spec, support 2 condition.

1. function call like var(), url(), linear-gradient()
2. url like 'test.png', "test.png"

[poyoho]

I think this needs to test inline CSS in HTML because it doesn't have url( and also needs to compile, may have had a problem with inline CSS in HTML.

[patak-dev]

Would you share a failing example to add to the PR?

[poyoho]

vite/packages/vite/src/node/plugins/html.ts

Lines 359 to 365 in 8e1d6ae

	const inlineStyle = node.props.find(
	(prop) =>
	prop.name === 'style' &&
	prop.type === NodeTypes.ATTRIBUTE &&
	prop.value &&
	prop.value.content.includes('url(') // only url(...) in css need to emit file
	) as AttributeNode

in here.

[patak-dev]

Nice! It is working though, are we good then?

[poyoho]

because I no add the assertion. 😂

[patak-dev]

@poyoho gotcha. Should the fix for this case be part of this PR, though? This wasn't working before even without var. I think we could merge this PR and then create a new PR or issue for inline style support, no?

[poyoho]

OK, I think this issue had two solutions. one is to remove the judge. other is add the judge. remove this judge may be better. But anyway it is also not part of this PR. (it seems to part of html instead of css).

[Bigfish8]

@patak-dev @poyoho The fail test seems not related, could you plz rerun it?

[patak-dev]

Thanks again @Bigfish8!

[qnp]

Thanks for the work 👍