-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
feat: allow .svg files to be inlined #1716
Conversation
Base64 is unnecessary for SVG files: https://github.com/vitejs/vite/issues/1197\#issuecomment-738780169 Also using `Buffer.byteLength` instead of character length when comparing with the `assetsInlineLimit` option.
|
url = `data:${mime.getType(file)};base64,${content.toString('base64')}` | ||
// svgs can be inlined without base64 | ||
url = file.endsWith('.svg') | ||
? `data:image/svg+xml;utf8,${content}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't escape unsafe characters. Might be worth taking a similar approach to svg-url-loader
? See Explanation, Usage and Implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data uri has a mime type, so I'm skeptical that the "unsafe characters" specification applies here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a URI, so yes you need to escape unsafe characters or it won't work.
E.g. for <path fill="#ffffff" />
, the #
starts the URL fragment and everything breaks, so it needs to be escaped to <path fill="%23ffffff" />
Hi @aleclarson, do you need a hand with this PR? |
@princed Sure, you can fork it and open another PR 👍 |
@aleclarson @nhardy @Shinigami92 |
Base64 is unnecessary for SVG files: #1197 (comment)
Also using
Buffer.byteLength
instead of character length when comparing with theassetsInlineLimit
option.Closes #1204