cascornelissen / svg-spritemap-webpack-plugin

SVG spritemap plugin for webpack
MIT License
211 stars 51 forks source link

Upgrade `svgo@2` to `svgo@3` or `svgo@4`? #214

Open dkrnl opened 6 months ago

dkrnl commented 6 months ago

Description This SVGO@2 is no longer supported.

cascornelissen commented 6 months ago

PRs for this dependency update are welcome, I don't have time myself the coming weeks but depending on the size of the changes I might be able to review a PR ❀️

cascornelissen commented 4 months ago

Looking at the release notes of svgo@3.3.2, it sounds like a v4 release is imminent. Since either v3 or v4 will likely result in a new major release of this plugin as well, I think it makes sense to wait until SVGO v4 is released. There's also a Release Candidate available already so I'd expect this to be available soonβ„’

xavierfoucrier commented 4 months ago

Hum, sorry to bother, but I just want to highlight the fact that using svgo@2 and svgo@3 with shared custom configuration between svg-spritemap-webpack-plugin and ImageMinimizerWebpackPlugin can lead to an incompatible state, especially when trying to deal with renamed options, like this one that was renamed in v3: https://svgo.dev/docs/plugins/cleanupIds/.

Waiting for v3/v4 update as well πŸ’―

xavierfoucrier commented 2 weeks ago

Hello @cascornelissen πŸ‘‹

Looks a Release Candidate has been recently publish on Github: https://github.com/svg/svgo/releases/tag/v4.0.0-rc.1

Do you have some time to have a look at this? Thanks!

cascornelissen commented 2 weeks ago

Thanks for the heads-up, I'm keeping an eye on it until there's a stable release. In the meantime, anyone is free to create a (WIP) PR to get the work started ❀️

xavierfoucrier commented 1 week ago

@cascornelissen πŸ‘‹

I have started working on this locally, but facing failing tests with the current master branch and NodeJS 20: is it normal that tests are failing or not currently? Thanks!

cascornelissen commented 1 week ago

@xavierfoucrier, nice that you want to take a look! Just pulled the latest commits from master myself, ran npm clean-install and npm test and all tests pass as expected. Running Node.js 22.6.0 at the moment but I can't imagine they'd fail on v20 πŸ€”

xavierfoucrier commented 1 week ago

Thanks @cascornelissen for your feedback on this, I have planned to continue tomorrow on that, will let you know πŸš€

xavierfoucrier commented 4 days ago

@cascornelissen another try on my side:

image

cascornelissen commented 2 days ago

@xavierfoucrier, could it be that you ran npm install at some point and you have changes to the lockfile on your end? What is the output of npm ls webpack? All tests pass on my end with webpack@5.73.0.

xavierfoucrier commented 1 day ago

@cascornelissen on my side:

And at this point, here is the return of npm ls webpack:

image

I have tested to downgrade to webpack@5.73.0 but it's clearly the same. I am working with Windows 11 Pro 64 bits.

Additional informations:

image