search

hudochenkov / postcss-sorting

PostCSS plugin to keep rules and at-rules content in order.
MIT License
516 stars 30 forks source link

Adds at-rule excludes for rules that can't be safely moved #83

Closed developering closed 6 years ago

developering commented 6 years ago

Hello!

As mentioned in #64, there are some at-rules that cannot be safely moved as part of the sorting process. I have run into this issue with @function. In that case, the @return statement can be moved in such a way that the function no longer works as expected. The reporter of #64 ran into it with control directives.

This change creates an array of excluded at-rules and filters those rules out of the sorting process. Tests all pass and I created one new test for the @function case. Let me know what you think!

fixes #64

developering commented 6 years ago

@hudochenkov Any thoughts on this change?

developering commented 6 years ago

@hudochenkov Thanks for the review! I incorporated those comments. I added a brief comment about why those at-rules are ignored, but otherwise went with your phrasing in the readme.

hudochenkov commented 6 years ago

Looking great!

I forgot one thing. Could you add a test, which includes ignored at-rule between sortable things, please?

div {
    display: block;

    @if (typeof($someCase) == number) {
        display: none;
    } @else {
        color: blue;
    }

    color: red;
}

You can add it right after test, that you added already.

developering commented 6 years ago

@hudochenkov I just added that test. I included a mixin call as well to show that the mixin does get moved to the top (as expected), but the @if does not.

developering commented 6 years ago

@hudochenkov Are there any other changes or additions that you would like to see?

hudochenkov commented 6 years ago

Thank you for your help!