Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

Consider switching compressor #94

Closed
MoOx opened this issue Apr 14, 2015 · 14 comments · Fixed by #127
Closed

Consider switching compressor #94

MoOx opened this issue Apr 14, 2015 · 14 comments · Fixed by #127

Comments

@MoOx
Copy link
Owner

MoOx commented Apr 14, 2015

https://github.com/ben-eb/cssnano

@MoOx MoOx modified the milestone: v2.0.0 Apr 14, 2015
@MoOx
Copy link
Owner Author

MoOx commented Apr 20, 2015

Or maybe we should just get rid of the compressor in cssnext.

@magsout
Copy link
Contributor

magsout commented Apr 24, 2015

it's more a clone of cssnext than a compressor ?

@MoOx
Copy link
Owner Author

MoOx commented Apr 24, 2015

Humm not from what i see in the first place. Only postcss-calc looks duplicated.

@magsout
Copy link
Contributor

magsout commented Apr 24, 2015

you're right. There are many optimization, code cleanup. So why not.

What's your idea ? Only use instead of csswring with all the options that seem interesting to activate by default? Or give the possibility to configure the options?

@MoOx
Copy link
Owner Author

MoOx commented Apr 24, 2015 via email

@magsout
Copy link
Contributor

magsout commented Apr 24, 2015

lol ok 👍

@magsout
Copy link
Contributor

magsout commented Apr 26, 2015

Two things:

  • options like sourcemap, calc, and urls are disabled by default? cssnext already using its function.
  • If I look at the documentation I see no option to disable the postcss-pseudoelements option, which is not great for cssnext

@MoOx
Copy link
Owner Author

MoOx commented Apr 27, 2015

Maybe @ben-eb can helps us on that ?

@ben-eb
Copy link
Contributor

ben-eb commented Apr 27, 2015

Sure; the architecture is similar to cssnext in that you can disable processors, although some of those I opted to not disable as they are safe optimisations. e.g since postcss-pseudoelements v2, the single colon syntax of before and after is supported across all browsers so is safe to minify.

Of course, I can add the option to disable these if you would like.

@MoOx
Copy link
Owner Author

MoOx commented Apr 27, 2015

It's not necessary since we are using this plugin only to ensure ie8 compat (so if IE 8 is not in the scope, we won't enable it).
Your arguments look good enough for me. Thanks.

@magsout
Copy link
Contributor

magsout commented Apr 28, 2015

@MoOx

It's not necessary since we are using this plugin only to ensure ie8 compat (so if IE 8 is not in the scope, we won't enable it).

yes, but cssnext consume cssnano after, so if IE 8 is not in the scope, cssnano doesn't matter and manage selector like after: and before: ?

And what do you think about sourcemap , calc ? We let users enable or disable those features ?

@ben-eb
Copy link
Contributor

ben-eb commented Apr 28, 2015

@MoOx @magsout I think you are better to wait until this attribute selector bug is fixed before you migrate. 😄

https://github.com/ben-eb/postcss-minify-selectors/issues/2

@MoOx
Copy link
Owner Author

MoOx commented Apr 29, 2015

@magsout you write ::. If the compression convert to : do you really care ? I don't think you should since you asked for the smallest possible size :)
sourcemap will need to be adjusted according to cssnext value.
calc should be disabled indeed.

@magsout
Copy link
Contributor

magsout commented Apr 29, 2015

LGTM

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

Successfully merging a pull request may close this issue.

3 participants