-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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(v2): use SVGO in webpack SVGR loader #3691
Conversation
Hi @charleskorn! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
I will have to get approval from my employer to sign the CLA - but please take a look at this (very small) change in the meantime and let me know if you foresee any issues. |
Deploy preview for docusaurus-2 ready! Built without sensitive environment variables with commit a882695 |
LGTM not sure why SVGO was disabled by default, hope it does not break anything for anyone, otherwise we'll provide a way to make this optim optional. |
I've just signed the CLA - hopefully this will come through shortly. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
This actually broke something in my documentation. With <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="24" height="24" viewBox="0 0 24 24">...</svg> However, with the version <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24">...</svg> The Should I open an extra issue for this? |
Yes @Dudrie please open an extra issue. We may want to allow to enable/disable SVGO through config. That could help if you give more details: your full SVG code before/after optimization and screenshots etc |
I have opened an issue for this. |
Motivation
Fix #3689.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Have verified changes on my own site, and scope of changes is small.