vite-svg-loader icon indicating copy to clipboard operation
vite-svg-loader copied to clipboard

Send path to svgo

Open mrleblanc101 opened this issue 1 year ago • 6 comments

Fix #60

mrleblanc101 avatar Jul 07 '22 17:07 mrleblanc101

Tested with npm link and work great 👍

mrleblanc101 avatar Jul 07 '22 21:07 mrleblanc101

Hi Sébastien, thanks for your PR! I'll look into it next week.

jpkleemans avatar Jul 08 '22 07:07 jpkleemans

Thanks

mrleblanc101 avatar Jul 08 '22 13:07 mrleblanc101

Actually I think path should be passed to svgo.optimize({ path: 'path.svg' }) alone and the options should be passed to new svgo(options) but you don't seem to instantiate the plugin like that.

mrleblanc101 avatar Jul 15 '22 17:07 mrleblanc101

Do you want me to make that change too ? See https://github.com/oliverfindl/vue-svg-inline-loader/blob/28051895a5db3c3beba6025963106566d4cee84a/src/index.js#L234

mrleblanc101 avatar Jul 15 '22 17:07 mrleblanc101

Thanks for your work. Could you review my comments? Also, is it possible for you to add a cypress test for this change?

I added one test

mrleblanc101 avatar Jul 18 '22 20:07 mrleblanc101

Actually I think path should be passed to svgo.optimize({ path: 'path.svg' }) alone and the options should be passed to new svgo(options) but you don't seem to instantiate the plugin like that.

In the svgo readme, it looks like you can add config to optimize just fine: https://github.com/svg/svgo#optimize

jpkleemans avatar Aug 31 '22 10:08 jpkleemans

Thanks for adding the test! Can you fix the merge conflicts? Then I can merge the PR

jpkleemans avatar Aug 31 '22 10:08 jpkleemans

@jpkleemans I fixed the merge conflicts.

mrleblanc101 avatar Aug 31 '22 20:08 mrleblanc101

Thank you!

jpkleemans avatar Sep 01 '22 13:09 jpkleemans