feat: add SVGO optimization support for SVG assets #13880
Conversation
github-actions
bot
added
pkg: astro
CodSpeed Performance ReportMerging #13880 will not alter performanceComparing Summary
|
🦋 Changeset detectedLatest commit: d8a9173 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks! This is a new feature, so we will need to discuss it a bit before merging, and it will need to wait for a minor release. There is a chance we will want to put this behind an experimental flag at first too. In the meantime, could you add a changeset? It's used to generate the changelog, so make sure you include enough detail to explain what it is and how it works for new users. Think of it as the first bit of docs. |
There was a problem hiding this comment.
Thanks for the PR @azat-io! I think this could be a nice feature.
Left a couple of questions based on a quick look.
Also pinging @natemoo-re and @stramel as our resident SVG experts who might have good insights here!
| svg: z | ||
| .object({ | ||
| optimize: z.boolean().default(true), | ||
| svgoConfig: z.record(z.any()).optional(), |
There was a problem hiding this comment.
I think we should offer at least proper typing here even if we don’t do full schema validation. For example, if a type is available for the SVGO options, we could use something like:
svgoConfig: z.custom<SVGOOptions>((value) => value && typeof value === 'object').optional(),That way users should get a nice editor experience.
| * @description | ||
| * Whether to enable SVG optimization using SVGO during build time. | ||
| * | ||
| * When enabled, all imported SVG files will be optimized for smaller file sizes |
There was a problem hiding this comment.
all imported SVG files
Will this also apply to usage with Astro’s assets pipeline? For example usage like:
---
import { Image } from 'astro:assets';
import mySVG from './example.svg';
---
<Image src={mySVG} alt="Example SVG" />
|
Thanks for the PR! I do have some initial thoughts on this. This approach is similar to what we did in astro-icon which had some downsides and issues. This might also conflict with an idea @natemoo-re had about shifting the icons into a content-collection instead. I was also thinking about alternatives to this approach that doesn't tie us so tightly to svgo. Similar to the image optimization setup or offering a optimize function/file. Sharp offers SVG optimization as well. Another thought was allowing opt-in per SVG though that can get a bit hairy. Let me know if you have any deeper thoughts. I'll keep thinking about it as well. Interested to hear @natemoo-re 's opinions as well |
azat-io
force-pushed
the
feat/svgo
branch
2 times, most recently
from
175772e to
f7104bc
Compare
|
Made fixes based on comments:
|
| svg?: { | ||
| /** | ||
| * @docs | ||
| * @name svg.optimize |
There was a problem hiding this comment.
| * @name svg.optimize | |
| * @name experimental.svg.optimize |
|
|
||
| /** | ||
| * @docs | ||
| * @name svg.svgoConfig |
There was a problem hiding this comment.
| * @name svg.svgoConfig | |
| * @name experimental.svg.svgoConfig |
|
Should I create a pull request in the documentation? |
|
Rebased. |
|
Any feedback? |
|
Ping. |
1 similar comment
|
Ping. |
|
Hey @azat-io! We’re waiting on getting some more feedback from our SVG experts — apologies for the delay. In the meantime, it would be good to have an answer to my question here: #13880 (comment) |
azat-io
force-pushed
the
feat/svgo
branch
2 times, most recently
from
446db9a to
8a6dd52
Compare
|
Rebased. |
|
Amazing stuff! I was looking for how to use svgo with astro today and came across this PR. Native support for svgo with a customizable svgo config would be extremely helpful. |
|
Hi @delucis, @stramel, @natemoo-re, Checking in - it's been 4 months since the last comment. All feedback has been addressed and the PR is ready for review. What's the status on this? |
5 participants
Changes
svg.optimizeandsvg.svgoConfigconfiguration optionsTesting
Added unit tests.
Benefits
Astro planned to add SVGO earlier:
#12067
Docs
I'll create another PR to docs.