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

Don't apply properties-alphabetical-order autofixing if there no violations #59

Open
pentzzsolt opened this issue Jan 22, 2018 · 7 comments

Comments

@pentzzsolt
Copy link

With the properties-alphabetical-order set to true, the following code will throw no errors:

.a {
  color: #000;
  padding: {
    bottom: 1em;
    top: 1em;
  }
  width: 25%;
}

However, the --fix flag will scramble the code to the following:

.a {
   color: #000;
   width: 25%;
   padding: {
      bottom: 1em;
      top: 1em;
   }
}

Stylelint version 8.4.0, stylelint-order version 0.8.0.

@hudochenkov
Copy link
Owner

This is happening because padding: {} treated as at-rule by SCSS parser. There is nothing I can do about it. I would suggest refactoring code to:

.a {
  color: #000;
  padding-bottom: 1em;
  padding-top: 1em;
  width: 25%;
}

@pentzzsolt
Copy link
Author

Unfortunately our team has a setup where we require nested properties via the declaration-nested-properties rule of stylelint-scss, so refactoring is not an option.

@hudochenkov
Copy link
Owner

Could you show your whole config, please?

@pentzzsolt
Copy link
Author

Well the entire package.json is rather long and somewhat confidental, but I removed everything, tested the following environment and successfully reproduced the behavior:

package.json:

{
  "name": "project",
  "version": "1.0.0",
  "private": true,
  "scripts": {
    "stylelint": "stylelint \"./**/*.scss\""
  },
  "devDependencies": {
    "stylelint": "^8.4.0",
    "stylelint-order": "^0.8.0"
  },
  "stylelint": {
    "plugins": [
      "stylelint-order"
    ],
    "rules": {
      "order/properties-alphabetical-order": true
    }
  }
}

styles.scss:

.a {
  color: #000;
  padding: {
    bottom: 1em;
    top: 1em;
  }
  width: 25%;
}

After running npm install, running npm run stylelint shows no errors, but npm run stylelint -- --fix will change styles.scss.

The strange thing to me in this is, if there are no errors, why will the --fix flag try to fix anything?

@hudochenkov
Copy link
Owner

The strange thing to me in this is, if there are no errors, why will the --fix flag try to fix anything?

Indeed, this shouldn't happen. Since 0.8.0 order and properties-order doesn't apply fixes if there were no violations. Somehow I missed properties-alphabetical-order while making this change.

Thank you for a report.

@pentzzsolt
Copy link
Author

I am more than happy to help!

@hudochenkov
Copy link
Owner

It would be great! You can check how I changed behavior for order 6b0c036.

For fix enabled. Old algorithm: fix and don't check for violations. New algorithm: check for violations. If there a violation — don't report it, set a variable shouldFix to true and skip all next checks, because we already know we should fix CSS. Run fix if shouldFix === true.

Feel free to ask any questions!

@hudochenkov hudochenkov changed the title Bug when autofixing properties-alphabetical-order with properties in nested group form in SCSS Don't apply properties-alphabetical-order autofixing if there no violations Feb 25, 2018
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

2 participants