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

Use encodeURIComponent instead of replace in tailwind config #6038

Open
kuon opened this issue Jan 6, 2025 · 5 comments
Open

Use encodeURIComponent instead of replace in tailwind config #6038

kuon opened this issue Jan 6, 2025 · 5 comments

Comments

@kuon
Copy link
Contributor

kuon commented Jan 6, 2025

Is there any reason to use replace instead of encodeURIComponent for svg url in tailwind config?

let content = fs.readFileSync(fullPath).toString().replace(/\r?\n|\r/g, "")

Isn't

                        let content = fs.readFileSync(fullPath).toString()
                        content = encodeURIComponent(content)

Better?

@josevalim
Copy link
Member

At least replace will make it shorter instead of encoding unwanted components?

@kuon
Copy link
Contributor Author

kuon commented Jan 8, 2025

At least replace will make it shorter instead of encoding unwanted components?

Yes it does. But I did change this in my code for a reason and I cannot remember why. It must have been an SVG with something that wasn't escaped in it, causing the resulting CSS to be invalid.

Maybe we can actually do both?

@josevalim
Copy link
Member

@kuon I am fine with calling encodeURIComponent but we would need to know why we are calling it. So if you can recall the particular SVG or example, that would be very helpful. :)

@kuon
Copy link
Contributor Author

kuon commented Jan 8, 2025

Ok, I found why I did that. I have some SVG with color in them, like fill="#e30000". And, at least under firefox, the # causes the icon to not be rendered if not encoded. I don't think that is a problem for hero icons as they all have fill="none" but I copied the tailwind config for my own icon set, and I use color in the SVG because I use them in two ways, sometime as tailwind style, as a mask, so no color, and sometimes as colored icons, as direct SVG.

I think it makes sense to change it and add encodeURIComponent even for hero icons, because if we consider the spec strictly, a data URI must be escaped. Also having it correct in the template makes it easier for dev to reuse it with their own icons, without having to understand why the icon stopped rendering after they did open it in inkscape and set a color.

@josevalim
Copy link
Member

So I think we can add encodeURICOmponent. A PR is welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants