-
-
Notifications
You must be signed in to change notification settings - Fork 425
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
Fix merging svgo config #384
Conversation
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/gregberge/svgr/5az8wdkxu |
Codecov Report
@@ Coverage Diff @@
## master #384 +/- ##
==========================================
+ Coverage 85.14% 85.52% +0.37%
==========================================
Files 30 30
Lines 505 518 +13
Branches 140 142 +2
==========================================
+ Hits 430 443 +13
Misses 62 62
Partials 13 13
Continue to review full report at Codecov.
|
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.
Looks good to me!
FYI this is a breaking change if you have an existing custom Webpack configuration. I had to update my config to fix it: use: [
{
loader: require.resolve('@svgr/webpack'),
options: {
svgoConfig: {
- plugins: { removeViewBox: false },
+ plugins: [{ removeViewBox: false }],
},
},
},
], |
Yeah but your configuration was not correct IMO, plugins should always be an array. |
Had the same issue. I also used this the config for the IMO, it would be a good idea to at least throw an error if an incorrect config is found. I can work on a PR if you'd like :) |
Yes some clear documentation on Webpack configuration would be helpful. |
Summary
The only optimization we need in our project is prefixing ids. But when we tried to use the configuration below, we faced the problem - prefixes were added unexpectedly to the class names.
Use https://react-svgr.com/playground for reproducing the issue:
This PR fixes the issue and allows to configure prefixIds plugin in more flexible way.
Test plan
New test case was added for plugin-svgo