Skip to content

Add support for svgo 2.x and 3.x#224

Merged
toy merged 1 commit into
toy:mainfrom
tomhughes:svgo-config
May 27, 2026
Merged

Add support for svgo 2.x and 3.x#224
toy merged 1 commit into
toy:mainfrom
tomhughes:svgo-config

Conversation

@tomhughes

Copy link
Copy Markdown
Contributor

This resolves #191 by generating a configuration file for svgo if the version is 2.x or higher.

@toy

toy commented Apr 12, 2026

Copy link
Copy Markdown
Owner

Thank you for opening the PR!
My biggest concern is that same config file will be created for every optimised svg file
Also some test would be nice

@tomhughes

Copy link
Copy Markdown
Contributor Author

I've modified the patch to memoise the config file so it's only created once.

As to testing it would be nice but I'm struggling to figure out how to test it... Are there even any tests that run svgo conversions? I can see there's a test svg file but I can't see what uses it? Even then I'm not sure how you'd test that this is working let alone testing the fallback path when we only have a current svgo.

hlfan added a commit to hlfan/openstreetmap-website that referenced this pull request Apr 19, 2026
to include the new revision of toy/image_optim#224
@toy

toy commented May 3, 2026

Copy link
Copy Markdown
Owner

I tried testing how it works and understood that current solution will work only for disabling plugins included in preset-default, as enabling not included plugins (for example removeScripts) will show a warning from svgo that they are not in the preset and should be added directly to plugins to define order.

Simple way to handle this is to just list all enabled plugins after preset-default in order, but I'm not sure if it is good enough, as plugins can (and some need to) be configured. Less restrictive and more precise (but also more involved/complicated) seem to be either allowing to specify complete config as a blob, which should be ok in yaml, but cumbersome as a CLI argument, or allowing to specify config path, which splits config into two files. Or even allow both.

What do you think?

@tomhughes

Copy link
Copy Markdown
Contributor Author

You're definitely right that enabled plugins should be listed after the defaults rather than as overrides in the default set, so I've made that change.

You're also right that in principle plugins may need to be configured, but I don't think that was possible before so as it stands I htink this now restores the legacy functionality? A future enhancement to allow plugins to be configured is definitely a possibility though.

@toy toy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I only caused a merge conflict when added a check of plugin names, so that it should be safe to output them unquoted in config

Comment thread lib/image_optim/worker/svgo.rb Outdated
Comment thread lib/image_optim/worker/svgo.rb Outdated
@tomhughes tomhughes force-pushed the svgo-config branch 2 times, most recently from 7eb51f7 to 68b5e32 Compare May 26, 2026 21:39
@toy toy force-pushed the main branch 2 times, most recently from 4ff340c to 40443d1 Compare May 26, 2026 23:06
@toy

toy commented May 27, 2026

Copy link
Copy Markdown
Owner

I understood that using tap doesn't close the file, so I add it myself and also a changelog entry

@toy toy merged commit d175a55 into toy:main May 27, 2026
20 checks passed
@toy

toy commented May 27, 2026

Copy link
Copy Markdown
Owner

Thank you!

@toy

toy commented May 27, 2026

Copy link
Copy Markdown
Owner

Included in 0.32.0

@tomhughes tomhughes deleted the svgo-config branch May 28, 2026 16:52
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

Successfully merging this pull request may close these issues.

Add support for SVGo 2.x

2 participants