Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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(astro): add Built-in SVG component support #12067
base: next
Are you sure you want to change the base?
feat(astro): add Built-in SVG component support #12067
Changes from 19 commits
aa3f606
0832ae3
5f128ea
434ceda
f8e94e3
2b6744b
a751408
11e2eba
3c576fd
3f274c4
a92cb57
66eec7a
fcc6eca
78490fb
ddf5604
767559c
31b08e7
c03a467
6ab0f1a
d40d45d
9344a25
28d6c38
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Would love @Princesseuh's take on this. Any expected footguns with including the file content for SVGs in the image metadata?
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.
If you have a lot of SVGs, this can be problematic for SSR since it'll get in your bundle, that's my only concerns
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.
Right. Unfortunately that will be unavoidable for this feature.
I think the DX improvement is worth the tradeoff, though!
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 definitely outside my realm of expertise. I'll defer to whatever the consensus is 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.
Something we could do one day (definitely outside of this PR), though it'd make SVG icons somewhat slower in SSR is add a lazy
data
property or something, that's essentiallyasync () => await fetch(this.src)
in SSR and something else in SSG so that you can load the data without it being added to the bundle.