n3-charts / line-chart

Awesome charts for AngularJS.
http://n3-charts.github.io/line-chart/
MIT License
1.2k stars 180 forks source link

\a in compiled css url #512

Closed darrinholst closed 7 years ago

darrinholst commented 8 years ago

build/LineChart.css has a few url declarations with inline svg containing \as...

    .chart-legend .item.dashed-line > .icon {
      background-image: url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' style='fill: white;' width='10' height='10'>\a         <g style=\"stroke: white; fill: none; stroke-dasharray: 4px,2px;\">\a           <path d='M0,6 L10,6'/>\a         </g>\a       </svg>"); }

From what I could tell it's from those selectors that use newlines like here.

I'm not sure what \a means (guessing it's a unicode escape string for a newline) and it wasn't a problem until a comical series of updates involving webpack, css-loader, postcss-modules-local-by-default and css-selector-tokenizer which eventually got to this code that converted them back to newlines which ended up turning my css in to...

    .chart-legend .item.dashed-line > .icon {
      background-image: url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' style='fill: white;' width='10' height='10'>
         <g style=\"stroke: white; fill: none; stroke-dasharray: 4px,2px;\">
           <path d='M0,6 L10,6'/>
         </g>
       </svg>"); }

in which any css after that point got ignored.

So I guess my question is there a way to have sass strip the new lines?

wips commented 7 years ago

@darrinholst, thanks a lot for pointing this out! I've just faced the same problem on my webpack + ExtractTextPlugin setup.

BTW, I've worked my issue around by adding purifyCCS plugin with minify: true option. Basically any minification would work, I believe.

darrinholst commented 7 years ago

I couldn't see a way for sass to do this natively, but I did find a way to do it with a function that would work. I can send a PR if you want.

diff --git a/src/styles/_legend.scss b/src/styles/_legend.scss
index ca204b5..7908d1a 100644
--- a/src/styles/_legend.scss
+++ b/src/styles/_legend.scss
@@ -1,5 +1,15 @@
+@function str-strip-newlines($string) {
+  $index: str-index($string, '\a');
+
+  @if $index {
+    @return str-strip-newlines(str-slice($string, 1, $index - 1) + str-slice($string, $index + 1));
+  }
+
+  @return $string;
+}
+
 @mixin svg($svg) {
-  background-image: url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' style='fill: white;' width='10' height='10'>#{$svg}</svg>");
+  background-image: url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' style='fill: white;' width='10' height='10'>#{str-strip-newlines($svg)}</svg>");
 }

 @mixin series-item {
lorem--ipsum commented 7 years ago

Wow, I'd never thought this could happen... The npm package contains both minified and regular CSS files, maybe using the regular one would help ?

darrinholst commented 7 years ago

iirc, the minified files have the newlines as well so that results in the same behavior. I ended up splitting my vendor css from the application css so I wouldn't have to do any additional processing on vendor code. It's not an issue for me anymore, but I think it would be good not to ship it with the newlines.

wips commented 7 years ago

@lorem--ipsum, I ended up string-replacing newlines with a webpack plugin, I would much appreciate a new version w/o newlines in npm