hudochenkov / stylelint-order

A plugin pack of order related linting rules for Stylelint.
MIT License
916 stars 61 forks source link

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

Closed pentzzsolt closed 1 month ago

pentzzsolt commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Could you show your whole config, please?

pentzzsolt commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

I am more than happy to help!

hudochenkov commented 6 years ago

It would be great! You can check how I changed behavior for order https://github.com/hudochenkov/stylelint-order/commit/6b0c03642e2114df32762a6392d394e6cc047ceb.

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!