svgo
svgo copied to clipboard
svg
Remove the `removeViewBox` plugin from the default plugins list
Given the discussions in #1128, we should remove the removeViewBox plugin from the list of default plugins.
The stated goal of svgo is to optimize SVGs while not changing their renderings. While SVGs can be rendered in many different contexts, one import context is rendering inside HTML documents. When an svg is placed inline with other HTML, removing the viewBox attribute (even when it is a duplicate of the data in the width/height attributes) does change how the svg is rendered.
According to the SVG 1.1 specification, section 7.7 The ‘viewBox’ attribute:
It is often desirable to specify that a given set of graphics stretch to fit a particular container element. The
viewBoxattribute provides this capability.
When svgo removes the viewBox attribute, it removes that SVG's ability to be scaled to fit its containing HTML element via CSS. In other words, the Scalable Vector Graphic is no longer scalable.
Here's a codepen with an extensive example: https://codepen.io/johnalbin/pen/yLggyXv You can see from the screenshot below that the SVG does not scale properly when CSS resizes the SVG without the viewBox attribute.
Almost all responsive web design websites use this (or a slight variant of the following CSS):
img,
svg {
max-width: 100%;
height: auto;
}
When that CSS is used, the browser treats the width and height attributes as the intrinsic size of the image as it scales the SVG canvas (what the SVG spec calls the viewport), but the contents of the SVG (what the spec calls the user space coordinates) are not scaled unless the viewBox attribute is present.
That means removing the viewBox attribute changes the rendering of the SVG. And, from the svgo homepage:
SVG files, especially those exported from various editors, usually contain a lot of redundant and useless information. This can include editor metadata, comments, hidden elements, default or non-optimal values and other stuff that can be safely removed or converted without affecting the SVG rendering result. (emphasis mine)
Which means the removeViewBox should not be included in the list of default plugins.
Here's the PR that fixes this bug.
Any update on this? It would be great to get this merged in.
I just finished adding back a bunch of missing viewBox attributes because svgo stripped them. I assumed svgo default settings would not strip anything important. This PR would have saved me a lot of time.
I noticed this as I was working through my little browser extension that uses SVGO and thought it was odd. The default configuration made it much less dependable to scale the svgs I'm scraping. I removed it from the "default config" as well.
+1 on this. Without the viewBox I can't scale the svg icons (while good scaling support is my point of using SVG icons). I'm surpised that they remove it by default for saving bits.
Meanwhile, this setting helps …
module.exports = {
plugins: [
{
name: 'preset-default',
params: {
overrides: {
removeViewBox: false,
},
},
},
],
};
I've updated the PR to match the code updates made in prep for 3.x releases.
@TrySound I've updated the initial PR description comment above to point out what the SVG 1.1 spec says about the viewBox attribute's importance.
I would very much like to see the removeViewBox plugin removed from the default preset. If other maintainers would like to create a new preset that uses that plugin, that's fine with me, but it really shouldn't be a part of the default preset for all the reasons stated above.
@TrySound could you also have a look at this PR?
We plan to disable the plugin in SVGOMG, but if you land this it would make more sense :)
Or a feature since it fixes important issues 🙂
Maybe a minor release would be OK then?
Breaking change smile
I doubt this change will break anything. Rather, it will do the opposite.
@TrySound This is a fix for a bug that prevents all SVGs from being scalable on all web pages.
So its a bug fix, not a breaking change feature request.
In my gulp-stacksvg I even have to replace width/height with viewBox, mainly because of the default svgo setting.
Breaking change 😄
it fixes things (not breaking it) and makes the svgo fully SVG-compliant
In my gulp-stacksvg I even have to replace
width/heightwithviewBox, mainly because of the default svgo setting.
I wasn't much aware of the viewBox property until the same thing happened, why would u remove it to save a few bytes?
while most projects are bloated with countless dependencies.
Since this project apparently now has a new maintainer, who was presumably the one who closed #1128, is any work being done to finally merge this PR?
Since this project apparently now has a new maintainer, who was presumably the one who closed #1128, is any work being done to finally merge this PR?
Arrogant maintainers won't ever merge this PR
Unless I meet any resistance, I'd be happy to merge this in future. Unfortunately… this will be a long-winded response due to the heated discussions surrounding the topic historically, especially as my stance differs from other maintainers.
Why won't I merge it now?
Based on the discussions in other issues, I'd have to treat this as a feature rather than a bug, so it is indeed a breaking change.
I would also like to give other maintainers time to contest this response if they have any qualms with it. This is important because if other maintainers disagree, it'll just be enabled again anyway. I would rather the project show a unified front than make SVGO a playground where maintainers with differing opinions override each other's changes.
With that in mind, I would like to merge this into v4.
Why wasn't it disabled sooner?
I think the problem stems from the mentality that we must reduce bytes at all costs. In my opinion, this isn't what an optimizer should be striving for. In fact, reducing bytes should be 4th or 5th priority.
In my opinion, an optimizer's priorities should be:
- Adhering to the spec. Even if clients allow us to do something, we probably shouldn't if it's not compliant with the spec.
- Adhering to the law. We must preserve copyright notices, licenses, and attribution by default.
- Preserving (not adding) accessibility, like
titleanddescription. Reducing bytes isn't worth discriminating against users. This also ties in with adhering to the law. - Preserving (not adding) compatibility, SVG 1.1 has more support than SVG 2, and we know this. That's why we still use the deprecated
xlinknamespace rather than use thehrefattribute available in SVG 2. - And finally… we can start reducing bytes where reasonable.
Optimizers aren't meant to "reduce bytes at all costs" because even for an optimizer, there are more important things than optimization.
Why do I agree to disable it?
After a user reported an issue with this plugin in 2013, Kir Belevich (the original author of SVGO) disabled the plugin himself. It was then silently enabled again 3.5 years later. There was a reason it was disabled, but I don't see a public discussion/reason it was enabled again, so that probably shouldn't have happened. ^-^'
The use case to disable this is extremely common. In fact, it's essential when using SVGs on responsive webites, hence most web toolkits disable the plugin by default. This is also where a substantial number of our NPM downloads stem from.
An optimizer should be safe by default, and the consensus is that removeViewBox isn't safe. I've also been bitten by this previously. Even if SVGO supports different modes/presets in future, I'd expect the safest mode to be the default mode.
It's clear that among users and engineers, this plugin should be off. Even large projects like Docusaurus and SVGR disable this by default for users, and I'd like to think Meta and Greg Bergé know what they're doing.
On Stack Overflow, a significant number of questions with the svgo tag are specifically looking for how to configure SVGO in the various ways to use it to disable removeViewBox. It's clear there is a demand for this.
As an open-source project making software for the community, we should value what the community has to say within reasonable measure. In this case, everyone has made overwhelming clear across mediums, directly and indirectly, that SVGO is handling this wrong, so we should step up to address it.
@SethFalco well said, and thank you for taking such a reasonable approach to this issue!
@SethFalco Thanks a lot! ❤️🔥
I think the problem stems from the mentality that we must reduce bytes at all costs. In my opinion, this isn't what an optimizer should be striving for. In fact, reducing bytes should be 4th or 5th priority.
Probably off-topic, but I hope this revision of the approach will someday touch the default of converting relative commands of the d attribute into absolute ones to save bytes. Now it breaks the ability to move the whole vector figure after optimization by shifting only the coordinates of the start point.
@firefoxic
Probably off-topic, but I hope this revision of the approach will someday touch the default of converting relative commands of the
dattribute into absolute ones to save bytes. Now it breaks the ability to move the whole vector figure after optimization by shifting only the coordinates of the start point.
Yeah, I'd say it's off-topic, but I'll respond here anyway. If you or anyone else wants to add more, I'd recommend opening a separate issue, and we'll continue the discussion there. (Or shoot me an email!)
That isn't something I favor myself because that impacts maintainability (human-editability) of the file, not the usability (rendering/scaling) of the vector.
Most optimizations in SVGO impact the maintainability significantly, and that applies to most other optimizers too. Whether that's absolute to relative coordinates, converting shapes to paths, inlining styles, omitting spaces between attributes or transforms, the SVG will inevitably become a hassle to maintain.
The main difference compared to other optimizers is that we've encouraged overwriting the SVG by documenting that as the primary usage and making it the default behavior of the CLI. That workflow is flawed though, considering it's annoying to edit after and if the SVG breaks, the original file is lost. 😨 (i.e. https://github.com/svg/svgo/pull/1461#issuecomment-853307980)
I was actually planning to add a warning to the documentation, that the output may be difficult to maintain, so to encourage treating the original SVG as the project file, and the output of SVGO as the final product. That's effectively what users of Docusaurus and SVGR are inadvertently doing, so they don't have that problem.
I don't want to rush into making drastic changes, but in future I do want to get feedback on if we should change the default behavior of the CLI to output to a new file called filename.min.svg instead of overwriting, to promote the separation. 🤔
PS: Instead of changing the start point, you could add a transform (translate) to the path. It'll be optimized away next time you use SVGO anyway.