-
Notifications
You must be signed in to change notification settings - Fork 49
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
frontend: replace vite-plugin-vue2-svg with vite-plugin-svg4vue #5173
Conversation
⛔ Feature branch deployment currently inactive.If the PR is still open, you can add the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because vite-plugin-vue2-svg does not support vue3. Only worked when converting vite.config.js to an esm file by renaming it to .mjs. vite-plugin-svg4vue is not the most popular package for this, this would be https://www.npmjs.com/package/vite-svg-loader, but that does not support vue2. Issue: ecamp#5121 We can also wait to merge this short before we want to upgrade to vue3. Also fixes https://github.com/ecamp/ecamp3/security/dependabot/178
use v-html, because outputting the raw svg code into the element does not work in production.
@@ -17,7 +17,9 @@ const plugins = [ | |||
VuetifyResolver(), | |||
], | |||
}), | |||
createSvgPlugin(), | |||
svg4VuePlugin({ | |||
skipSvgo: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What does svgo do? Why do we want to disable it?
- I think the option key should be
svgoConfig
, at least according to the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It minimizes the svgs. I'm not sure this is the way to go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we want to use this plugin when we can just replace the plugin once we move to vue3? The vue3 one looks like a drop in replacement for our current one
I am not a fan of big rewrites. |
I vote for vite-svg-loader. It is already used in the print package and is a drop in replacement for vite-plugin-vue2-svg |
But not compatible with vue2 |
Core Meeting DecisionWe want to try, if the vite-svg-loader is a drop in solution, otherwise we will try this PR again. |
Because vite-plugin-vue2-svg does not support vue3. Only worked when converting vite.config.js to
an esm file by renaming it to .mjs.
vite-plugin-svg4vue is not the most popular package for this, this would be https://www.npmjs.com/package/vite-svg-loader, but that does not support vue2.
Issue: #5121
We can also wait to merge this short before we want to upgrade to vue3.
Also fixes https://github.com/ecamp/ecamp3/security/dependabot/178