googlearchive / polyclean

BSD 3-Clause "New" or "Revised" License
20 stars 7 forks source link

Specific CSS broken after vulcanizing #8

Closed ebidel closed 8 years ago

ebidel commented 8 years ago

From @omarbelkhodja on December 3, 2015 16:15

I'm using polybuild to vulcanize my app, before deploying on the webserver. My website is using a Polymer paper-dialog that uses the following CSS code:

.shadow-elevation-16dp {
  box-shadow: 0 16px 24px 2px rgba(0, 0, 0, 0.14),
              0  6px 30px 5px rgba(0, 0, 0, 0.12),
              0  8px 10px -5px rgba(0, 0, 0, 0.4);
}

After vulcanizing, this code is transformed into the following:

.shadow-elevation-16dp {box-shadow: 0 16px 24px 2px rgba(0, 0, 0, 0.14),06px 30px 5px rgba(0, 0, 0, 0.12),08px 10px -5px rgba(0, 0, 0, 0.4);}

There are some missing spaces, (see 06px instead of 0 6px, and 08px instead of 0 8px).

Of course, this is leading to shadows not displayed.

Copied from original issue: PolymerLabs/polybuild#21

ebidel commented 8 years ago

https://github.com/PolymerLabs/polyclean/blob/master/index.js#L144 looks like it blindly removes 2 or more spaces, which would explain your example.

ebidel commented 8 years ago

I'm going to move this to polyclean.

ebidel commented 8 years ago

cc @azakus

omarbelkhodja commented 8 years ago

May I suggest something like the following :

.replace(/ +/g, ' ')
.replace(/([,:;!\(\)\[\]]) /g, '$1')

instead of

.replace(/ {2,}/g, '');
indolering commented 8 years ago

Wouldn't it be safer to use a dedicated CSS processing library?

indolering commented 8 years ago

In my case, HTML/CSS only make up about 10% of the final output, so the maximum HTML/CSS minification could save is 10%. Comparing gzipped output, whitespace removal shaved .5K from 310KB of code, or about .16%.

This is a sample size of one, but I would suggest just ripping this out until you can do a proper job.

ebidel commented 8 years ago

@azakus why not use clean-css with most of the advanced optimizations off?

rubenstolk commented 8 years ago

I guess it is better to use a well-tested library although I can confirm that with some fixes (see PR #9) it all works brilliantly now. I've tested it in a fairly large project that also uses many of the iron and paper elements.

@omarbelkhodja I saw your comment after the PR but I think my PR is on similar lines as your suggestion.

omarbelkhodja commented 8 years ago

I saw this issue https://github.com/PolymerElements/polymer-starter-kit/issues/564 And @robdodson is saying that Polymer Team is working on a real CSS parser. So probably this will be fixed when CSS parser is ready, but might require sometime before beeing available...

robdodson commented 8 years ago

FWIW this issue, PolymerElements/polymer-starter-kit#564, was related to a plugin called minifyInline https://github.com/PolymerElements/polymer-starter-kit/pull/565/files#diff-b9e12334e9eafd8341a6107dd98510c9L196

rubenstolk commented 8 years ago

@azakus I think this can be closed with #9 and #10

dfreedm commented 8 years ago

Probably fixed with #10, please file a new issue if this is still broken.