Improved error message for properties in groups

[mbrowne]

Currently, when using property groups, you'll get an error message like this if your code doesn't match the specified order:

Expected "top" to come before "background"

It would be nice if the error message mentioned the fact that the property is also expected to be part of a group.

Here's an example use case...at my company we are using styled-components, so we can't use the --fix option. So we want to require an order for properties that only requires remembering a few rules and is also readable. This is what we want to use:

'order/properties-order': [
  {
    properties: ['position', 'top', 'right', 'bottom', 'left']
  },
  // remaining properties in alphabetical order
  'align-content',
  'align-items',
  // etc...
]

Given these rules, if a new developer starts working on a project and puts a rule, e.g. position or right in alphabetical order instead of at the top:

padding: 10px;
right: 0;

They'll get errors like this:

Expected "right" to come before "padding"

It would be great if the error message wouldn't just say that right should go above padding, but that it belongs in the positioning group.

I created a fork of this repo that achieves this (note: I didn't add a Jest test for it yet):
https://github.com/mbrowne/stylelint-order/tree/group-error-message
mbrowne@6ba7e43

It allows property groups to have an optional name property:

'order/properties-order': [
  {
    name: 'positioning',
    properties: ['position', 'top', 'right', 'bottom', 'left']
  },

And if a group name is present, the error includes it, e.g.:

Expected "right" to be grouped with other "positioning" properties and to come before "padding"

I think that would be much more helpful for developers new to our rules than the current error.

Does this sound like a good solution? Should I submit a PR? Thanks.

[hudochenkov]

Here's an example use case...at my company we are using styled-components, so we can't use the --fix option.

I believe you can use it if you use stylelint without stylelint-processor-styled-components. stylelint support CSS-in-JS from a box.

I think that would be much more helpful for developers new to our rules than the current error.

Sounds good!

I checked your code and I think it should be located in the other file, but I can't say for sure. (Which indicates, that I'm probably need to refactor my code :)) I'll dig into it, and tell you later next week (I'm traveling right now).

If you participating in Hacktoberfest, then feel free to open “work in progress” pull request, so your contribution will count.

[jeddy3]

Expected "right" to be grouped with other "positioning" properties and to come before "padding"

Whilst it's on my mind, I'll just add a suggestion for the message itself. How's about?:

Expected "${first}" to come before "${second}" in group "${group}"

It'll be more aligned with the messages of the built-in rules because they follow the convention of making sense regardless of what goes in the quotes. For example:

'order/properties-order': [
  {
    name: 'one',
    properties: ['position', 'top', 'right', 'bottom', 'left']
  },
  {
    name: 'two',
    properties: ['color', 'background']
  }
]

Would give:

Expected "right" to come before "padding" in group "one".

Whereas the current implementation would give:

Expected "right" to be grouped with other "one" properties and to come before "padding"

Which, arguably, makes less sense as '"one" properties' isn't a thing.

[mbrowne]

Great - thank you, no rush.

Whilst it's on my mind, I'll just add a suggestion for the message itself. How's about?:

Expected "${first}" to come before "${second}" in group "${group}"

Good catch. I was aiming for a message that would make sense both in this case:

padding: 10px;
right: 0;

And in this one, where the property is already in the correct group but not in the right order within that group:

right: 0;
position: 10px;

I'm not sure if the in group "${group}" part might be misleading in the first case, but I think this is a minor point. Probably still an improvement over my version.

[mbrowne]

Also: I'm not participating in Hacktoberfest, but I went ahead and created a PR labeled "WIP":
#73

[mbrowne]

@hudochenkov

I believe you can use it if you use stylelint without stylelint-processor-styled-components. stylelint support CSS-in-JS from a box.

I just looked into this...thanks, but it looks like you need a processor of some sort to enable this:

Processors can enable stylelint to lint the CSS within non-stylesheet files. For example, you could lint the CSS within <style> tags in HTML, code blocks in Markdown, or strings in JavaScript.
https://stylelint.io/user-guide/configuration/#processors

And stylelint does not currently support autofixes for processors. See:
stylelint/stylelint#2592
styled-components/stylelint-processor-styled-components#50

Autofixing would be a great feature, but I don't think it would be possible without stylelint adding support for this to the processor API. And it could be complex for styled-components because it the template string can also contain JS functions.

[hudochenkov]

@mbrowne stylelint supports linting and autofixing in template literals since 9.4.0 without preprocessors.

[jeddy3]

Processors can enable stylelint to lint the CSS within non-stylesheet files. For example, you could lint the CSS within <style> tags in HTML, code blocks in Markdown, or strings in JavaScript.
https://stylelint.io/user-guide/configuration/#processors

This is my bad. I missed this bit about processors in the configuration doc when I deemphasised processors elsewhere in the docs after the 9.4.0 release.

I've created a quick PR to fix the configuration doc: stylelint/stylelint#3747.

As @hudochenkov said, linting and fixing of styled-components is supported out-of-the-box now.