Rule Proposal vue/attribute-order

[mikegreiling]

Please describe what the rule should do:

Provide a template rule to enforce placing vue-specific attributes before other properties in a template tag.

What category of rule is this? (place an "X" next to just one item)

[X] Enforces code style
[ ] Warns about a potential error
[ ] Suggests an alternate way of doing something
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about:

Configuration

'vue/closing-bracket-location': ['alphabetical' | 'vue-first']

Examples:

// bad
<a
  v-tooltip
  class="btn external-url"
  data-container="body"
  target="_blank"
  rel="noopener noreferrer nofollow"
  :title="title"
  :aria-label="title"
  :href="externalUrl">

// good
<a
  v-tooltip
  :title="title"
  :aria-label="title"
  :href="externalUrl"
  class="btn external-url"
  data-container="body"
  target="_blank"
  rel="noopener noreferrer nofollow">

[mikegreiling]

I'd be open to a better name for this rule 😄

[kaicataldo]

I wonder if more fine-grained control could be added to the config. I'm thinking something similar to to the config options for eslint-plugin-import's order rule.

[michalsnik]

I very much like this idea, plus it have already been mentioned in #77

[michalsnik]

https://vuejs.org/v2/style-guide/#Element-attribute-order-recommended is the best point of reference how the config should look like :)

[erindepew]

@michalsnik @mikegreiling I just opened a PR for this rule #209 is this what you had in mind?

[matt-oconnell]

Hi @michalsnik @mikegreiling, thanks for all the hard work that's already gone into this plugin. I've been working to add configuration to this rule with @erindepew. Before moving forward, we just want to ensure that we're not doing duplicate work and that we're moving in the right direction.

1. As mentioned in Style Guide Collaboration #77 there are recommended groupings of attributes in the style guide: https://vuejs.org/v2/style-guide/#Element-attribute-order-recommended. I was thinking of accepting an array of group names as a configuration option to define the order. If none are given, the rule will enforce the default order specified within the style guide.

2. The could also be an alphabetical option. This would enforce alphabetical order within groups themselves.

3. Error messages: we would error on the first invalid attribute and report "Attribute {name} of group {group} should come before attribute {name} of group {group}"

Let me know if it seems like we're moving in the right direction.

[mikegreiling]

That sounds good to me 👍

[michalsnik]

Yeah @matt-oconnell @erindepew, you seem to go in the right direction, just to sum up:

1. It should be possible to set order of the groups (I'm not sure about alphabetical order within groups to be honest, as often it's not so intuitive, maybe only inside the group of the other attributes)
2. Default order should be the one defined in styleguide.
3. Regarding errors you can check: https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/order-in-components.js
   I already worked on similar rule, but regarding component properties :)

Example config could look like:

'vue/attributes-order': [2, {
  order: [
    'CONDITIONALS',
    'RENDER_MODIFIERS'
    // ...
  ],
  alphabetical: true,
}]

But I also started to wonder if it wouldn't be even better to be more explicit and give users the ability to amend the order of groups as well using 2d arrays:

'vue/attributes-order': [2, {
  order: [
    ['v-show', 'v-cloak', 'v-if', 'v-else-if', 'v-else'],
    ['v-once', 'v-pre'],
    'OTHER_ATTRS',
    // ...
  ],
}]

We could even combine those two ideas, and initially transform settings with group names to be replaced by the explicit arrays of attributes, and then perform all checks on it. It would be possible to easily override order in specific groups instead of the whole configuration.

What do you think?

[kaicataldo]

As a starting place, seems like having the current groupings specified by the style guide could be a good place to start. Adding the ability to create custom groupings would be a non-breaking change and could easily be added in the future, and could make it easier to land this and get the rule out in the wild sooner.

My biggest concern with this rule is the error messaging and having to keep track of such a long list of arbitrarily ordered groupings. Autofixing is great, but I think the warning message should be useful in its own right, since not everyone autofixes when writing a file (I know I don't!).

As an example, what happens when someone turns this rule on for an existing codebase that doesn't follow the enforced order at all? Will it only report one attribute at a time, making the user fix each order change operation individually? I think that would be a frustrating experience for any elements that have a lot of attributes.

As a proposed solution (and hopefully there's a better one), I wonder if we could notify the user of the overall grouping order in the error message? I'm envisioning something along the lines of an error message that says where the first offending node should be moved to and then "the expected order for attributes is CONDITIONALS, RENDER_MODIFIERS, etc.". This would allow the user to see the error and potentially fix up any other errors they see without having to refer to the config to see the full desired order.

Finally, if we do this, I think custom groups should also be named so that they can also be included in the error message:

'vue/attributes-order': [2, {
  order: [
    {
      name: 'CUSTOM_CONDITIONALS',
      group: ['v-show', 'v-cloak', 'v-if', 'v-else-if', 'v-else']
    },
    {
      name: 'SOME_OTHER_CUSTOM_GROUP',
      group: ['v-once', 'v-pre']
    }
    'OTHER_ATTRS'
  ],
}]

It's a bit verbose, but the above configuration could then yield the following information in the error:
"the expected order for attributes is CUSTOM_CONDITIONALS, SOME_OTHER_CUSTOM_GROUP, OTHER_ATTRS"

[robsterlini]

Was #170 (comment) ever taken forward? Have just updated to 4.4.0 and am implementing this as a new rule we have, but have found it's not flexible enough. Was wondering if it's worth disabling this in the meantime if this was likely to be done?

[erindepew]

@robsterlini not that I know of, but I think that @kaicataldo proposed solution would certainly work and I'm happy to open up another PR with the enhancement. What do you think @michalsnik?

[michalsnik]

No, it hasn't been taken forward yet. Feel free to submit PR with proposed changes - so we can challenge it :)

Another idea for possible settings:

'vue/attributes-order': [2, {
  groups: {
    CUSTOM_CONDITIONALS: ['v-show', 'v-cloak', 'v-if', 'v-else-if', 'v-else'],
    SOME_OTHER_GROUP: ['v-once', 'v-pre'],
  },
  order: [
    'CUSTOM_CONDITIONALS',
    'SOME_OTHER_GROUP',
    'OTHER_ATTRS'
  ],
}]

I think it would be even easier to set-up :)

[n4ks]

At the moment there is still no way to set the order of the attributes more accurately? For example : [class, v-for, v-if, etc.]