search

cascornelissen / svg-spritemap-webpack-plugin

SVG spritemap plugin for webpack
MIT License
207 stars 49 forks source link

<symbol> id is mangled by SVGO but not the css url anchor #fragment #179

Closed Diono closed 2 years ago

Diono commented 2 years ago

Description Since the last release, all my added <symbol> tag have id mangled but the css bacground-image url dont use the mangled name to aim the <symbol> tag

Expected behavior css background url use the mangled name : <symbol id="a"> => background: url(sprite.abcd.svg#a) 50% no-repeat;

Actual behavior css background url dont use the mangled name : <symbol id="a"> => background: url(sprite.abcd.svg#toto-fragment) 50% no-repeat;

i will make a test project to reproduce... but if you are any ideas about this issue already, it's be cool :)

Diono commented 2 years ago

here we are : https://github.com/Diono/test It's because of SVGO cleanupIDs option. When active, SVGO mangle the id in the emitted file.

cascornelissen commented 2 years ago

It's technically not possible to use styles with fragments and have the cleanupIDs plugin enabled.

It looks like a minor bug snuck in with the 4.3.1 release where the plugin was overwritten correctly but then added as a separate plugin too, so it became enabled again.

Can you verify the fix in d9a8be0 by installing the plugin directly from Github?

npm install cascornelissen/svg-spritemap-webpack-plugin#master
Diono commented 2 years ago

indeed the fix works in the sense that it no longer allows to mangled the IDs by SVGO... It is a shame on the other hand that we cannot reduce a little the weight of its svg sprite and the css by minimizing all ids :/ Couldn't we add an option to svg-spritemap-webpack-plugin that would do it instead of SVGO?

cascornelissen commented 2 years ago

Thanks for reporting back so quick. Great to hear at least the implementation is working as before now, I've published a patch release, 4.3.2, to npm 🚀

I guess it would be possible for the plugin to handle that, it's kind of an edge case and I'm afraid it will increase the complexity of the project by quite a bit so I'm not 100% sold on it. Especially because setting up compression (e.g. gzip) on the server takes care of 99% of this problem already. Then again, if anyone thinks it's feasible, I'm open to a pull request of course ✌🏼

I'm closing this issue for now as the actual bug has been fixed but feel free to comment further to discuss the above.