vitejs / vite

Next generation frontend tooling. It's fast!
http://vite.dev
MIT License
68.35k stars 6.16k forks source link

Major browsers (Chrome, Firefox...) just removed data URLs in SVG elements, resulting in a difference betweend dev and prod #15453

Open hugoattal opened 10 months ago

hugoattal commented 10 months ago

Describe the bug

Article: https://developer.chrome.com/blog/migrate-way-from-data-urls-in-svg-use?hl=en Chrome status: https://chromestatus.com/feature/5128825141198848

Vite seems to inline SVGs when building a project. But data URLs in SVG don't work anymore (cf article).

So there's a difference between dev (when there's no inlining, everything work) and prod (when it's inlined, it does not display anything)

Reproduction

https://stackblitz.com/edit/vitejs-vite-meuqwh?file=main.js&terminal=dev

Steps to reproduce

pnpm run dev

The rendered SVG element looks like this:

<use href="/src/vite.svg#logo"></use>

=> Works as intended

image


pnpm run build && pnpx serve dist

The rendered SVG element looks like this:

<use href="data:image/svg+xml,%3csvg%20xmlns ... 13Z'%3e%3c/path%3e%3c/g%3e%3c/svg%3e#logo"></use>

=> Does not work

image

System Info

Firefox last version (122.0b3) and Chrome last version (122.0.6182.0)

Used Package Manager

pnpm

Logs

No response

Validations

stackblitz[bot] commented 10 months ago

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

hugoattal commented 10 months ago

For anyone stumbling upon this issue, here's an easy way to solve it in the meantime:

Before:

import './style.css';
import viteLogo from '/src/vite.svg';

document.querySelector('#app').innerHTML = `
  <div>
  <svg width="256px" height="256px">
    <use href="${viteLogo}#logo"></use>
  </svg>
  </div>
`;

After:

import './style.css';
import viteLogo from '/src/vite.svg?raw';

document.querySelector('#app').innerHTML = `
  <div>
  <div style="display:none;">${viteLogo}</div>
  <svg width="256px" height="256px">
    <use href="#logo"></use>
  </svg>
  </div>
`;
hugoattal commented 10 months ago

I'm willing to tackle the issue, but I'm not exactly sure how to proceed...

There's already this logic:

https://github.com/vitejs/vite/blob/56ae92c33cba6c86ab5819877c19b9ea39f7121b/packages/vite/src/node/plugins/asset.ts#L350-L351

But fragments can be added after importing the file (just like in my example)

I think there can be two solutions:

The second solution does not fix the problem of difference between dev and prod, though. So we could have the same logic of inlining in the dev mode, but that might break reactivity on SVGs...

If we default to inline, the problem can be undetected before build. If we default to file, build will be good by default, but this will change the behavior of what vite already does.

Here's how this could be solved:

 // Don't inline SVG with fragments, as they are meant to be reused 
 (!(file.endsWith('.svg') && (id.includes('#') || id.endsWith('?file'))) && 

cf https://github.com/hugoattal/vite/commit/0e9b6361c210293b743acc24da098cee2da84db2

Wdyt?

sapphi-red commented 10 months ago

TIL data URLs now disallowed in <use>. There's a PR that adds a option to exclude some files to be inlined: #15366 I guess that PR will work for this case as well.

hugoattal commented 10 months ago

TIL data URLs now disallowed in <use>.

You can't imagine the headache to find out why my icons were not displayed anymore, a browser update was at the veeeeery bottom of my list 😅...

There's a PR that adds a option to exclude some files to be inlined: https://github.com/vitejs/vite/pull/15366

I think the ?file suffix would be a bit more practical, but I'm okay with the solution of this PR 🙂

Terbiy commented 8 months ago

Hello! Also faced the issue recently. Is there any news on the topic?

aloayzab88 commented 2 months ago

Also faced this problem recently. assetsInlineLimit: 0 fix the issue but... well you would not inline anything.