fix(utils): keep the url when a density descriptor omits its leading zero - #23346
fix(utils): keep the url when a density descriptor omits its leading zero#23346NgoQuocViet2001 wants to merge 2 commits into
Conversation
…zero
A density descriptor may be written without its leading zero -- `.5x`
is as valid as `0.5x`. The descriptor group started with `\w`, which a
dot does not match, so the group failed and the url alternation matched
the descriptor instead:
'./nested/asset.png .5x, ./nested/asset.png 1x'
-> [{ url: '.5x', descriptor: '' }, { url: './nested/asset.png', descriptor: '1x' }]
The real url is dropped from the candidate list entirely, so base-URL
rewriting and asset resolution operate on '.5x' and the first candidate
points at nothing.
Allow a dot as the descriptor's first character.
|
The regex change is a no-op on every real fixture in the repo. I extracted all 14 distinct That includes the base64 candidates, the Unit tests are unaffected. On this branch vs unmodified Same 26 failures either way — all in Same base, different result. #23345 branches from the same commit and passed that identical Windows job, 20/20. So I have no evidence this change caused it, and some evidence it did not — but the job log is not readable without an authenticated token, so I can't name the failing test to be sure. If someone can re-run the Windows job, that would settle it; if it reproduces, please paste the failing test name and I'll dig in properly rather than guess. |
| // The descriptor may start with a dot: a density descriptor is allowed to omit | ||
| // its leading zero (`.5x`). Without the dot in the leading class the descriptor | ||
| // group fails, the alternation then matches the descriptor itself as the url, | ||
| // and the real image url is dropped from the candidate list. |
There was a problem hiding this comment.
| // The descriptor may start with a dot: a density descriptor is allowed to omit | |
| // its leading zero (`.5x`). Without the dot in the leading class the descriptor | |
| // group fails, the alternation then matches the descriptor itself as the url, | |
| // and the real image url is dropped from the candidate list. |
Don't need to explain a bug
There was a problem hiding this comment.
Removed in 17fca76 — dropped the whole four-line block, not just the suggested line, since the rest of it was explaining the same bug. The existing comment above the regex is untouched.
2 participants
Description
A density descriptor in
srcset/image-set()may be written without its leading zero —.5xis as valid as0.5x. When one is,parseSrcset()discards the image URL and returns the descriptor in its place.Because the URL never reaches the candidate list, base-URL rewriting and asset resolution run on
.5x, and the first candidate ends up pointing at nothing:Quoted URLs are mangled the same way, and worse — the quote handling desynchronises:
Why
The descriptor group starts with
\w:A dot is not
\w, so the optional group fails to match. The url alternation[^,]\S*[^,]then happily consumes.5xas the url, and the actual path is left out of the match.The fix allows a dot as the descriptor's first character.
[^,]+still requires two or more characters, which every legal descriptor satisfies (1x,.5x,400w,600dpi) — that constraint is pre-existing and unchanged.Validation
Both new tests fail on
mainand pass with the change:npx vitest run packages/vite/src/node/__tests__/utils.spec.ts— 124 passed (124).processSrcSetSyncblock through the old and the new pattern — base64, comma-in-URL, no-descriptor,url()-with-commas,400w,600dpi, quoted URLs,1.5x— and the parsed output is identical in all of them. The only inputs whose behaviour changes are the ones that were broken.npx eslintandnpx oxfmt --checkon both files — clean.Same code path as the
image-set()handling inplugins/css.ts, so that gets the fix too.