-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
chore: tailor svgo config for inline svgs #10205
Conversation
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
On the side, it may be nice to enable other SVGO plugins. We have a few plugins that are disabled by default, but perfectly acceptable to use for inlined vectors. Reference: https://github.com/svg/svgo.dev/blob/main/src/plugins/configure-svgo.js#L26
Have any thoughts on turning these on (in a separate PR)? |
Ahh, sorry. I can see a PR like this was already submitted and rejected. I'll shelve this, and later look into a good way to configure SVGO from Docusaurus. SVGO does have an If this PR catches your eye, I'd appreciate feedback on the comment above still! |
I support adding some method for configuring SVGO. I ran into the duplicate IDs problem referenced in issue 8297. Every time I add a new SVG to a page where IDs are already in use I have to go back and update empty IDs and refactor my stylesheet. |
Pre-flight checklist
Motivation
Users have encountered issues when multiple SVGs are inlined in the same page, which is a very common scenario, SVGO's cleanupIds plugin can introduce conflicting IDs after optimization.
Even without cleanupIds, SVGs are typically maintained separately and may happen to have conflicting IDs, which can be problematic when they use CSS selectors. I have checked to see if there's anything we could do better in SVGO, but nothing comes to mind without introducing state which doesn't seem worthwhile.
Imo, the only lasting solution to this is for tools that foresee this scenario to enable prefixIds. It's worth noting, SVGR actually enables prefixIds by default, but Docusaurus incidentally disables it when disabling removeViewBox and removeTitle.
I configured prefixIds in this PR to use a custom delimiter and prefix:
delim
- because there's no need for it.prefix
- this part of your webpack configuration only applies to files ending with.svg
, so there's no benefit to including that suffix in the prefix.Test Plan
Before:
After:
Test links
Deploy preview: https://deploy-preview-10205--docusaurus-2.netlify.app/tests/docs/tests/svgs/
Related issues/PRs