Skip to content
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

Vite adds spaces after commas in image url within srcset (HTML files) #16323

Closed
7 tasks done
rarous opened this issue Mar 31, 2024 · 3 comments · Fixed by #18242
Closed
7 tasks done

Vite adds spaces after commas in image url within srcset (HTML files) #16323

rarous opened this issue Mar 31, 2024 · 3 comments · Fixed by #18242
Labels
feat: html p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@rarous
Copy link
Contributor

rarous commented Mar 31, 2024

Describe the bug

Vite appears to add spaces around commas when a comma is present in a srcset property. This only occurs in HTML files.

If my index.html file contains

<img
srcset="https://res.cloudinary.com/hackercamp/image/upload/dpr_1,f_auto,fl_progressive,q_auto,w_100/v1/www.hackercamp.cz/faq-ubytko-3 1x, https://res.cloudinary.com/hackercamp/image/upload/dpr_2,f_auto,fl_progressive,q_auto,w_100/v1/www.hackercamp.cz/faq-ubytko-3 2x"
src="https://res.cloudinary.com/hackercamp/image/upload/dpr_auto,f_auto,fl_progressive,q_auto,w_100/v1/www.hackercamp.cz/faq-ubytko-3"
>

Vite outputs

<img srcset="https://res.cloudinary.com/hackercamp/image/upload/dpr_1, f_auto, fl_progressive, q_auto, w_100/v1/www.hackercamp.cz/faq-ubytko-3 1x, https://res.cloudinary.com/hackercamp/image/upload/dpr_2, f_auto, fl_progressive, q_auto, w_100/v1/www.hackercamp.cz/faq-ubytko-3 2x" src="https://res.cloudinary.com/hackercamp/image/upload/dpr_auto,f_auto,fl_progressive,q_auto,w_100/v1/www.hackercamp.cz/faq-ubytko-3">

Take note of the space added to the image URL after the comma in the pathname. /dpr_2, f_auto, fl_progressive, q_auto, w_100/.

To narrow the matching logic, in Cloudinary case, the commas always between slashes.

Reproduction

https://stackblitz.com/edit/vitejs-vite-ruqamh?file=index.html

Steps to reproduce

HTML plugin always breaks it

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.3 - /usr/local/bin/pnpm
  npmPackages:
    vite: ^5.2.6 => 5.2.7

Used Package Manager

npm

Logs

No response

Validations

Copy link

stackblitz bot commented Mar 31, 2024

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@caseycarroll
Copy link
Contributor

Thanks for providing this use case. It's one I hadn't considered when fixing #16081.

Now having spent some time in the srcset parsing code, the solution to your use case is not immediately obvious to me. The parser uses a regex to strip out patterns that contain commas, replaces those patterns with spaces, then adds the pattern back into the string after each srcset has been split by a ,. These regex patterns match characters containing commas that supersede something specific. https://github.com/caseycarroll/vite/blob/1cb74f8944abc344fc37d180671d605031b8fe4f/packages/vite/src/node/utils.ts#L772

In my case, I stripped out characters that were in the middle of a word, superseded a ? and contained a ,. For your case, the pattern isn't as clear to me since image URLs can be relative paths.

There is the srcset package, but it's output doesn't align with what Vite expects. Vite wants: [{url: 'some url', descriptor: '100vw'}] and this package returns:

 [
  { url: "hifi-cat.jpeg",  density: 3 },
   { url: "lowfi-cat.jpeg", width: 128 },
 ]

So, some paths forward are:

  • figure out a pattern that will account for this use case in the existing parseSrcset function
  • leverage srcset package and add code to convert things like { url: "hifi-cat.jpeg", density: 3 }, to { url: "hifi-cat.jpeg", descriptor: '3x' },
  • write a new parseSrcset function modeled after the srcset package's solution

@rarous
Copy link
Contributor Author

rarous commented Apr 1, 2024

In my case, the URLs are always with protocol, never relative. But it should be generic; otherwise you will end up with other cases also broken. The srcset package uses regex conforming to the HTML spec and is much simpler than special cases in the current solution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: html p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants