Skip to content
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

"p{foo:bar;foo:baz}" and "p{foo:bar}p{foo:baz}" are minified differently #532

Closed
pygy opened this issue Apr 18, 2015 · 5 comments
Closed

Comments

@pygy
Copy link

pygy commented Apr 18, 2015

This may or may not be intentional, but I was surprised by the following:

> c.minify("p{foo:bar;foo:baz}").styles
'p{foo:bar;foo:baz}'

> c.minify("p{foo:bar}p{foo:baz}").styles
'p{foo:baz}'

If I pass {advanced:true} {advanced:false} to the constructor, the second example produces "p{foo:bar}p{foo:baz}" instead.

I'm using clean-css to normalize style sheets to test a JS to CSS compiler (probably an anathema for designers, I know), but I can't get it to behave accordingly in this case, where my code generates "p{foo:bar}p{foo:baz}".

@jakubpawlowicz
Copy link
Collaborator

This is in fact intentional, as take the following two declarations:

a{display:inline-block;display:block}
a{display:inline-block}/* some other properties */a{display:block}

In the first one, you declare two display properties adjacent to each other so it's highly likely you want to do a fallback, when one is not understood (more on this in a second) then the other one is picked. It's a common pattern in CSS.

In the 2nd one these are in two different selectors so it's less likely the author intention was to have a fallback, thus the display:block overrides display:inline-block.

However this is far from perfect so we started merging properties based on understandability, where less understandable properties (take color:rgba(...) which IE<9 does not support) cannot be overridden by more understandable, e.g. color:red. This is already done for background, color, marging, padding, and others: https://github.com/jakubpawlowicz/clean-css/blob/master/lib/properties/compactable.js#L35

Ultimately it will be implemented for all properties, see #290, but you can get a safer behavior using aggressiveMerging: false in the meantime.

Hope this helps!

@pygy
Copy link
Author

pygy commented Apr 18, 2015

That behavior makes sense when processing hand-crafted CSS, which is not my use case.

j2c is dumb, and things that are close together in the JS source may end up in consecutive but separate declarations for the same selector.

I ended up switching to crass which produces the same output for both inputs.

@jakubpawlowicz
Copy link
Collaborator

You can get the same behavior in clean-css by using advanced: false flag.

@pygy
Copy link
Author

pygy commented Apr 18, 2015

My bad, I meant advanced:false in the first post, and it doesn't suit my use case because it still outputs two rule sets rather than one with both properties. I could have changed the reference output in my test to match it, but in the long run, I wanted a CSS normalizer, which produces identical output for semantically equivalent, but syntactically distinct input, and clean-css doesn't do that.

A casting error on my side :-)

@jakubpawlowicz
Copy link
Collaborator

Ok, makes sense now! I'll close the issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants