Pavliko / postcss-svg

PostCSS plug-in which to insert inline SVG to CSS and allows you to manage it colors.
http://pavliko.github.io/postcss-svg/
Creative Commons Zero v1.0 Universal
124 stars 23 forks source link

SVG can't be processed if viewBox is not defined. #43

Open spiltcoffee opened 5 years ago

spiltcoffee commented 5 years ago

What did I do? I was trying to run postcss-svg over a FontAwesome .css file (see https://use.fontawesome.com/releases/v5.8.1/css/all.css for an example).

It has a bunch of @font-face definitions at the end of the file. Each font face has an .svg file in its list of sources, referenced using an #id. For example:

@font-face {
  font-family: "Font Awesome 5 Brands";
  font-style: normal;
  font-weight: normal;
  font-display: auto;
  src: url(../webfonts/fa-brands-400.eot);
  src: url(../webfonts/fa-brands-400.eot?#iefix) format("embedded-opentype"),
    url(../webfonts/fa-brands-400.woff2) format("woff2"),
    url(../webfonts/fa-brands-400.woff) format("woff"),
    url(../webfonts/fa-brands-400.ttf) format("truetype"),
    url(../webfonts/fa-brands-400.svg#fontawesome) format("svg"); /* <-- SVG font file -->
}
.fab {
  font-family: "Font Awesome 5 Brands";
}

What did I expect? That postcss-url, which was placed after postcss-svg in my postcss pipeline, to stop complaining about having to inline an entire SVG file (it can't handle SVG fragments).

What happened instead? postcss-url kept complaining. postcss-svg apparently silently failed? (I'm not sure if there's a way to get it to report errors, but I resorted to some good old console.logging to nail this one down).

What do I think happened? These FontAwesome SVG fonts do not define any viewBox attributes.

When rebuilding the SVG fragment, there is an attempt to copy the viewBox to the new SVG document: https://github.com/Pavliko/postcss-svg/blob/ad46505e3b20a84734ff6ed786341f2019a0c596/src/lib/element-as-data-uri-svg.js#L16

I think in this case the viewBox is then ending up undefined, which means a later attempt to call toString on the attribute falls over.

I was able to validate this by appending || "" to the line above, which caused everything to then work as I expected.

jonathantneal commented 4 years ago

If there is no viewBox attribute, I should omit it entirely.