hail2u / node-css-mqpacker

Pack same CSS media query rules into one using PostCSS
675 stars 60 forks source link

Plugin generate not equal styles #55

Closed SuperOl3g closed 6 years ago

SuperOl3g commented 6 years ago

The merging of queries means reordering of selectors. So we will have not equal stylesheet on output.

Example:

.myClass {
    font-size: 50px;
}

@media (max-width: 1024px) {
    .myClass {
        font-size: 40px;
    }
}

.myClass_big {
    font-size: 76px;
}

@media (max-width: 1024px) {
    .myClass_bigOnMobile {
        font-size: 76px;
    }
}

After using of plugin code will look like this:

.myClass {
    font-size: 50px;
}

.myClass_big {
    font-size: 76px;
}

@media (max-width: 1024px) {
    .myClass {
        font-size: 40px;
    }
    .myClass_bigOnMobile {
        font-size: 76px;
    }
}

In the first case, element <div class="myClass myClass_big">Text</div> have font-size: 76px at any window width. In the second case, block

@media (max-width: 1024px) {
    .myClass {
        font-size: 40px;
    }
}

have moved down, so selector with default styles for mobile devices now have higher priority than selector with modificator big. Therefore, text on phones will have font-size: 40px, not font-size: 40px.

SuperOl3g commented 6 years ago

As a result, the plugin can create a lot of bugs and it can't be used!

illarionvk commented 6 years ago

This is the expected behavior of the plugin.

Using min-width media queries would yield more consistent results.

Input:

.myClass {
  font-size: 40px;
}

@media (min-width: 1025px) {
  .myClass {
    font-size: 50px;
  }
}

.myClass_big {
  font-size: 60px;
}

@media (min-width: 1025px) {
  .myClass_big {
    font-size: 80px;
  }
}

Output:

.myClass {
  font-size: 40px;
}

.myClass_big {
  font-size: 60px;
}

@media (min-width: 1025px) {
  .myClass {
    font-size: 50px;
  }

  .myClass_big {
    font-size: 80px;
  }
}
SuperOl3g commented 6 years ago

@illarionvk yes, we can change CSS so that the plugin doesn't break anything. But if we have a big project, we can't just rewrite the whole code base.

SuperOl3g commented 6 years ago

Ohh... my bad, my problem has already been described in README.md. But I still don't understand what is the benefit or convenience of this plugin if we have to adjust the code to the rules of the plugin. It's not easier than merging media-queries by yourself at writing phase. It's too hard for ~5% size reduction (before gzipping).

Merging of media-queries is incorrect as a concept. Because DOM-elements can have several classes and it's almost impossible to understand how it's associated with each other, so we can't change the priority of CSS-selectors by reordering.

hail2u commented 6 years ago

In pure CSS world, if some elements have classes that change same property (font-size, for example), one of these classes is meaningless (never used) and broken.

In min-width (aka mobile-first) and component world, sometimes overriding property with multiple classes. It can be a problem. But components ALWAYS come first (and should be), so packed queries keep order as defined. There is no problem on cascading order.

If you don’t think it is useful, don’t use it.