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

SVG don't escape single quotes with url('') in css #15378

Open
7 tasks done
geminway92 opened this issue Dec 18, 2023 · 7 comments
Open
7 tasks done

SVG don't escape single quotes with url('') in css #15378

geminway92 opened this issue Dec 18, 2023 · 7 comments
Labels
has workaround p2-to-be-discussed Enhancement under consideration (priority)

Comments

@geminway92
Copy link

Describe the bug

Hello, I am waiting for the fixes with svg and the conversion of base64. #15271
I installed the last release 5.0.10 and integrated this pull that fixed my problem but It stopped working.
In the next images show how I import icon, how I use icon in tag video importing with URL the CSS, and how to build icon in mode compile.
Thanks very much beforehand for your help.

Describe the bug
I'm using svelte
I'm using vite 5.0.10
The last version vite that broke this 5.0.0
The last version vite that work is 4.5.0

Import the icon in svg
image

Use in tag div
image

How build my icon in production
image

Sorry, I didn't send the link for reproduction because Stackblitz doesn't allow me to reproduce a fine example.

Reproduction

https://no-example.com

Steps to reproduce

No response

System Info

System:
    OS: macOS 14.2
  Binaries:
    Node: v16.18.0 - /usr/bin/node
    npm: 8.19.2 - /usr/bin/npm
  Browsers:
    Chrome: 120.0.6099.109 (Build oficial) (arm64)

  npmPackage:
    vite: 5.0.10

Used Package Manager

npm

Logs

No response

Validations

@chaejunlee
Copy link
Contributor

chaejunlee commented Dec 19, 2023

How build my icon in production
image

I saw the image above, and I can see that it might cause trouble. There are nested single-quotes within url property. Which means there are three levels of quotes.

I think the encoding itself isn't the problem here. I think the problem is that the style that has three level deep quotes is being inlined.

Maybe in this case, it can be solved by not inlining the style property. I would like to have some opinions of others and am more than happy to elaborate further.

And it would be great if you can provide reproduction as much as you can.

@geminway92
Copy link
Author

How build my icon in production
image

I saw the image above, and I can see that it might cause trouble. There are nested single-quotes within url property. Which means there are three levels of quotes.

I think the encoding itself isn't the problem here. I think the problem is that the style that has three level deep quotes is being inlined.

Maybe in this case, it can be solved by not inlining the style property. I would like to have some opinions of others and am more than happy to elaborate further.

And it would be great if you can provide reproduction as much as you can.

Thanks for answering me. I can't do this. It is a problem that has occurred since 5.0.0 version.

@chaejunlee
Copy link
Contributor

If that's the case, can you roll back the version of vite to where your problem doesn't appear and provide that build result?

At this point, I can't tell what's wrong and elaborate further.

@geminway92
Copy link
Author

This is the 5.0.10 version. Although this has occurred since version 5.0.0

production mode:
It doesn't work here.
Captura de pantalla 2023-12-20 a las 8 59 06

development mode:
Here it works
Captura de pantalla 2023-12-20 a las 9 02 15

This is the 4.5.0 version. This version works fine.
production mode:
Captura de pantalla 2023-12-20 a las 9 16 47

development mode:
Captura de pantalla 2023-12-20 a las 9 19 26

@chaejunlee
Copy link
Contributor

chaejunlee commented Dec 20, 2023

I think your issue has more to do with this PR #14643. Not super sure but it looks like SVGs are inlined as default.

As mentioned in my PR, you can try the following to solve the issue.

export default defineConfig({
  build: {
    assetsInlineLimit: 0,
  },
})

Link to the documentation as well

@bluwy
Copy link
Member

bluwy commented Dec 27, 2023

This seems to be a limitation with the SVG inlining heuristic as we inline SVGs that contain the ' character. So when you manually add that to url() in style tags, you need to avoid using ' instead. Plain url(...) without quotes should work.

Unfortunately I don't think this is something we can fix. If we want to be conservative, we'd have to always base64 it and that isn't best for compression. Maybe we should document the caveats.

@bluwy bluwy added discussion p2-edge-case Bug, but has workaround or limited in scope (priority) and removed pending triage labels Dec 27, 2023
@chaejunlee
Copy link
Contributor

A lot of people seem to encounter this svg inlining problem. I've already came across 3 people out in the wild who got helped by the "double-quote" solution. Until there is a improvement on url() heuristics, I think we should have some info in the documentation. I have made a PR and would like to get any feedback on it! Thanks!!

@patak-dev patak-dev added has workaround p2-to-be-discussed Enhancement under consideration (priority) and removed discussion p2-edge-case Bug, but has workaround or limited in scope (priority) labels Feb 12, 2024
jeremywiebe added a commit to Khan/perseus that referenced this issue Apr 1, 2024
## Summary:

Mark noticed that the new @phosphor icons were not showing up in Storybook on Github Pages (`khan.github.com/pereseus`). 

I tracked it down to a feature introduced in Vite 5 (vitejs/vite#15534), but it seems like it has bugs ([#15378](vitejs/vite#15378) and [#15444](vitejs/vite#15444)). 

I don't see how we could "quote" the URL as it's handed of to Wonder Blocks which seems to be handling the import correctly. 

So instead we'll just force Vite to never inline SVGs using Vite's `assetsInlineLimit` (https://vitejs.dev/config/build-options#build-assetsinlinelimit) option. 

Issue: LEMS-1887

## Test plan:

I haven't tested on GH Pages yet, but I did the following and verified that there are now svg files in the build output folder:

```sh
yarn build-storybook
ls storybook-static/assets
```

Author: jeremywiebe

Reviewers: benchristel, jeremywiebe, mark-fitzgerald

Required Reviewers:

Approved By: benchristel

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Extract i18n strings (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has workaround p2-to-be-discussed Enhancement under consideration (priority)
Projects
None yet
Development

No branches or pull requests

4 participants