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

Autofix conflict between order and properties-alphabetical-order #129

Closed
runfaj opened this issue Oct 6, 2020 · 5 comments
Closed

Autofix conflict between order and properties-alphabetical-order #129

runfaj opened this issue Oct 6, 2020 · 5 comments

Comments

@runfaj
Copy link

runfaj commented Oct 6, 2020

I have the following setup for my order rule:

"plugins": [
    "stylelint-scss",
    "stylelint-order",
],
"order/order": [
      "custom-properties",
      "dollar-variables",
      "at-variables",
      "less-mixins",
      "at-rules",
      "declarations",
      "rules",
],
"order/properties-alphabetical-order": true,
"at-rule-empty-line-before": ["always", {
      "except": ["after-same-name", "first-nested"],
      "ignoreAtRules": ["include", "extend"]
}],
...

This works perfectly when viewing a file in an editor like vscode - marking the errors in all the right places.

However, when I do a stylelint fix, the ordering doesn't seem to match. Taking this code block for example, that was run after a lint fix:

input[type=text] {
        -webkit-border-radius: 4px;
        -moz-border-radius: 4px;
        border-radius: 4px;
        padding: 10px 0 11px 7px;
        width: 40%;
        @include theme(background, backgroundLight, url('images/search_b.svg') 96% 10px no-repeat);
        @include themeBorder();
        @include theme(box-shadow, borderLight, inset 0 0 10px);

        &:focus {
          outline: 0;
        }
}

You can see the alphabetical was applied, but not the order config. I would expect the final output to look like this:

input[type=text] {
        @include theme(background, backgroundLight, url('images/search_b.svg') 96% 10px no-repeat);
        @include themeBorder();
        @include theme(box-shadow, borderLight, inset 0 0 10px);

        -webkit-border-radius: 4px;
        -moz-border-radius: 4px;
        border-radius: 4px;
        padding: 10px 0 11px 7px;
        width: 40%;

        &:focus {
          outline: 0;
        }
}

It seems like the order config doesn't get fully applied within blocks. Is this expected, is there a config I might be missing to fix this, or a potential bug/feature that needs updating in the package?

@hudochenkov
Copy link
Owner

The last example does follow your config. Maybe I don't understand the issue you're facing. Does still showing lint errors? Does produce result that you're not expecting?

Could you show input code, and output code after --fix? Does still produce lint errors? What do you expect?

@runfaj
Copy link
Author

runfaj commented Oct 12, 2020

@hudochenkov Here's some screenshots so you can also see the red underlined errors.

Before --fix:

image

After --fix:

image

Here's the error happening due to the order config not being applied:

image

And here's the expected order that doesn't cause errors when I manually change it.

image

You can see that if I manually change it to have @include above the regular css properties, it matches the rule order mentioned in the issue description. However, the stylelint fix doesn't seem to be applying the order/order rule (it still applies the alphabetical), so it complains about there still being an error.

Also, for verbosity, I took out the "order/properties-alphabetical-order": true, rule from my config and left the order/order rule. Here was the result on the same block of code:

image

This screenshot also appears to demonstrate the order/order rule isn't being applied, since we've got the width attribute still appearing above the @includes.

@hudochenkov
Copy link
Owner

It happens, because less-mixins is not supported by postcss-sorting. #71

If you remove this keyword everything will work as expected.

@runfaj
Copy link
Author

runfaj commented Oct 12, 2020

@hudochenkov Magic! I didn't think to look at that, since I would have assumed it would be ignored in the processing if not a valid rule (like the other issue you linked to mentions). Thanks!

@runfaj runfaj closed this as completed Oct 12, 2020
@hudochenkov
Copy link
Owner

It should have show a message like other users pointed in #71. Maybe in VS Code it was suppressed.

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