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

postcss-merge-rules can be unsafe in certain conditions #41

Closed
kizu opened this issue Jul 31, 2015 · 5 comments
Closed

postcss-merge-rules can be unsafe in certain conditions #41

kizu opened this issue Jul 31, 2015 · 5 comments

Comments

@kizu
Copy link

kizu commented Jul 31, 2015

Merging rules with common declarations can be unsafe when the selectors have different browser support:

a{color:blue}b:nth-child(2){color:blue}

becomes

a,b:nth-child(2){color:blue}

While the support for those selectors is different, so if you'd merge them, IE8 won't render anything, while in the non-optimized case it would render the first rule.

@ben-eb
Copy link
Collaborator

ben-eb commented Jul 31, 2015

Good call. What do you think about making this optional for those consumers who don't want IE8 support?

@kizu
Copy link
Author

kizu commented Jul 31, 2015

Ideally, there should be a way to detect such things for any optimization and then look at the general browser support option (like for autoprefixer) decide what to use. There could be a lot of optimizations that are possible only for a limited set of browsers, or which could possibly break something for older browsers, so it should be a single framework that could be used through all of the cssnano plugins.

@ben-eb
Copy link
Collaborator

ben-eb commented Oct 14, 2015

@choffmeister
Copy link

We found another situation, where this rule is unsafe. Consider this less source:

.list-unstyled {
  list-style-type: none;
  margin: 0;
  padding: 0;
}

ul {
  .list-unstyled;
  margin-top: 10px;

  li {
    .list-unstyled;
  }
}

This yields the following css source:

.list-sorting-item ul {
  list-style-type: none;
  margin: 0;
  padding: 0;
  margin-top: 10px;
}

.list-sorting-item ul li {
  list-style-type: none;
  margin: 0;
  padding: 0
}

After minifying it with postcss-merge-rules one gets:

.list-sorting-item ul {
  margin-top: 10px;
}

.list-sorting-item ul,.list-sorting-item ul li {
  list-style-type: none;
  margin: 0;
  padding: 0
}

Due to reordering now the margin: 0; again overwrites the margin-top: 10px.

@ben-eb
Copy link
Collaborator

ben-eb commented Feb 29, 2016

@kizu If you cannot be bothered to review the functionality which I wrote for you back in October, I guess that I cannot help you with this issue.

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

No branches or pull requests

3 participants