Eslint-plugin-vue: Style Guide Collaboration

[chrisvfritz]

I'm developing an official Vue style guide and although not all rules there will be enforceable through this plugin, I think it makes sense to collaborate here, especially for those rules that _are_ enforceable. Right now, I'm thinking the style guide could serve as the official documentation for any recommended rules in this plugin and these two resources could be published together.

You can see the current progress in this branch, with the live result here. Any feedback/contributions are welcome and rules are not set in stone, so don't panic if you see something you disagree with. 😄

I've also not kept up-to-date with the conversation in various issues here, so I apologize if some of it is out of sync with plans you've been making. Please ping me on discussions you think I should know about, or if you'd like my input.

[armano2]

👍 I like it

[mysticatea]

Good idea ❤️
I think good if we have predefined configs about the priority A to D.
Also, the style guide is the best source of rule ideas.

[chrisvfritz]

@mysticatea Agreed, though inspiration can also go both ways. If you have any rules from the plugin that should be added, or if a recommended rule in the plugin conflicts, please discuss it here.

Feedback on the categories I've created are also welcome - as well as feedback on which category each rule belongs in. I tried to put a lot of research and thought into them, but this is still the first community style guide I'm creating and I'm sure there are considerations I'm not experienced enough to foresee.

[chrisvfritz]

@filipalacerda Also tagging you here, as I know you've thought about this quite a lot at Gitlab and your internal style guide was actually a source of inspiration for the official community one. 🙂

[sdras]

This is amazing! I love it.

A couple of notes: (these are _so_ minor)

Complex computed properties
Whenever possible, computed properties should be split into as many named values as possible.

I think this one could do with an example.

Component style scoping
Component styles should be scoped.

Clarification needed: I know a good number of people who won't scope the topmost App.vue component tag, but every subsequent component thereafter, are we deciding that this is an antipattern too? If not, it might be worth detailing this a bit more.

Self-closing components
Components with no content should be self-closing in single-file component or string templates, but never in DOM templates.

Can you please explain the inconsistency here?

This is fantastic, I'm sure it's going to be a great resource for people.

[chrisvfritz]

@sdras Great comments! I just updated those three entries to (hopefully) resolve your concerns. Let me know how I did. 😄

[sdras]

Perfect! Thank you.

[filipalacerda]

💪 Love this!
GitLab style guide goes mainly in the same direction - https://docs.gitlab.com/ce/development/fe_guide/style_guide_js.html.

Can't wait to have a linter to check them instead of doing manually in every merge request!

Some suggestions:

Props Casing

A rule for pros name casing in templates, similar to "Component name casing in templates", it is something we struggle with in our codebase, we often see:

<component myProp="prop"></component> ```` and

```
We created an entry to use the latter: https://docs.gitlab.com/ce/development/fe_guide/style_guide_js.html#naming

name property casing

We haven't decide on a rule to this one which makes us have components without it, other with it sometimes in camelCase others in PascalCase.
Might be worth adding a rule in priority B or C to aim for consistency

{
  name: 'MyComponent'
}

{
  name: 'myComponent'
}

Let's get this done 💪 I would love to help!

[armano2]

@filipalacerda can you make tickets with new rule proposition and i will work on this tonight (i have no idea how to name them).

[shentao]

When it comes to templates I prefer having the attributes in such order: directives -> attrs -> :attrs -> @events.

Example:

<input 
  v-for="name of names" 
  v-model="name" 
  type="text" 
  :disabled="isDisabled(name)" 
  @change="validate(name)"
/>

Could also order based on the native attributes first and directives/props/events second.

[filipalacerda]

@armano2 created #92 and #93 :)

[chrisvfritz]

@filipalacerda I like both of those ideas. 🙂 I'll also add a rule about max attributes per line and take another look at Gitlab's style guide to see what else you added since I last looked at it.

[chrisvfritz]

I just released a beta of the Vue style guide, with plans to remove the beta tag in mid to late October.

@mysticatea @armano2 @michalsnik Do you think that could be a good time for an official v3 release of this plugin? By that time, I'm thinking we could ensure that:

• vue/recommended is consistent with rules in the style guide
• when an ESLint rule has an equivalent in the style guide, we link to the style guide rule - and also link to the ESLint rule from the style guide

Thoughts?

EDIT: Note that I do _not_ expect the ESLint plugin to be able to enforce all style guide rules. I'd just like to be able to communicate to users what they can expect to be enforced here and what they will have to enforce manually. For rules that are _technically_ enforceable by ESLint, but would take a while to develop, it might be best to wait on those to ensure we can publish what we have.

[michalsnik]

Hey @chrisvfritz , I saw a tweet about it already. Very good work! 💪 🚀

I think we can do that. I was quite busy recently thus didn't have enough time to push things here but it looks like we have only 3 things left to do that block us before the official release, and some of them or all are already addressed and wait for review which I'm going to do very soon. In the meanwhile we can discuss the potential config, so it's in line with the styleguide and also update documentations.

I'll better focus on pushing forward everything that waits in queue in official release milestone first. And if you're able to prepare a basic PR with proposed configuration on which we can further iterate that'd be really great :) If not, I'll probably finally have some time during upcoming weekend, so I can do that too.

[chrisvfritz]

And if you're able to prepare a basic PR with proposed configuration on which we can further iterate that'd be really great

Will do. 🙂

[fatihacet]

@shentao Re: https://github.com/vuejs/eslint-plugin-vue/issues/77#issuecomment-316083699

Another GitLab dev here and at GitLab we were discussing to have Vue related attributes before the native attributes. It makes it easier to see the Vue attributes at a glance, so you don't need to skim through all attributes to find out whether that element already has the Vue attribute you are looking for.

<input 
  v-for="name of names" 
  v-model="name"
  :disabled="isDisabled(name)" 
  @change="validate(name)"
  type="text" 
  placeholder="Name"
/>

[piehei]

@fatihacet that's what I've been doing too. Enforcing this would be nice.

[chrisvfritz]

@fatihacet @piehei We actually have a rule addressing attribute order in the style guide. I'd love your feedback. 🙂

[kaicataldo]

Hi! Where do things stand with this? Is there a checklist or something to look at to see what is done and work is still outstanding? Looking forward to this!

Also, apologies if this has been discussed somewhere else, but are there plans to create different sets of rules for the different tiers of importance (Essential, Strongly Recommended, etc.) or is the plan to just add it to the recommended set right now?

[matt-oconnell]

@chrisvfritz I started work with @depew on an eslint rule for attribute order. I wonder if the attribute order is too fine-grained. In working with Vue, we've often enforced an order more like:

1) refs
2) directives (anything with a v- prefix excluding bind)
3) event listeners (@)
4) bound props (:)
5) generic attributes (href, src, class...)

10 grouping types seems a little much to remember but that's just my own opinion. With the above ordering, we don't need to remember the subsets but rather we can simply group attributes with matching prefixes together. Maybe @fatihacet @piehei @kaicataldo have an opinion on this?

[chrisvfritz]

are there plans to create different sets of rules for the different tiers of importance (Essential, Strongly Recommended, etc.) or is the plan to just add it to the recommended set right now?

@kaicataldo #205 creates these rule sets.

10 grouping types seems a little much to remember

@matt-oconnell While that grouping is easier to remember, I don't think it's very useful, as so many of the most important distinctions are lost. For example, :is would appear near the bottom in group 4, making it easy to miss, but it completely changes the context for every other attribute. A v-if would also be allowed to appear before a v-for, which could imply that they're interpreted in that order.

Either way, I think this is something that should be fixable, which means no one will even have to remember it if they use this plugin. 🙂

[michalsnik]

@chrisvfritz I've been looking at max-attributes-per-line rule today and I wonder whether the default value for singleline option shouldn't be 2 or 3, not 1. Consider the following common case:

<div v-for="n in 10" :key="n">
</div>

I would expect this example to not throw a warning by default. What do you think?

[michalsnik]

I'm going through the styleguide and will add more rules' propositions that we could implement in next releases. I'll mark them with label Present in Style Guide and it would be our next priority after releasing stable 4.0.

We should also reprioritise other propositions in favor of the official style guide now :)
Please take a look guys at all propositions and comment, so we can decide together what do we want to add and what not. Maybe there is something even worth considering adding to official style guide @chrisvfritz :)

And actually I think that the flow for adding new rules in this repository should include decision process on the style guide level. If something is worth adding to official style guide - that means it's definitely worth the hassle to make a rule for it too. Do you agree?

I'll create another label Style Guide candidate for marking propositions that we should consider adding to the style guide first.

[chrisvfritz]

I'm going through the styleguide and will add more rules' propositions that we could implement in next releases. I'll mark them with label Present in Style Guide and it would be our next priority after releasing stable 4.0.

That sounds like a good plan to me. 🙂 We also still have to add links back and forth between the style guide and rules in this repo.

We should also reprioritise other propositions in favor of the official style guide now :)
Please take a look guys at all propositions and comment, so we can decide together what do we want to add and what not.

I agree that enforcing rules in the style guide should take priority, as that will probably be what users expect.

Maybe there is something even worth considering adding to official style guide

New additions to the style guide page are definitely welcome!

There will be some rules we can't/shouldn't enforce here, such as those typically in _Priority D: Use with Caution_ - how do we detect if a feature was _not_ used with caution? 😄

And similarly, some rules here will be implied rules in the style guide, but not explicitly mentioned, such as no-dupe-keys. That one's probably obvious and straight-forward enough so that no justification/exploration is needed, but it's a great ESLint rule because it could be easy to make the mistake.

And actually I think that the flow for adding new rules in this repository should include decision process on the style guide level. If something is worth adding to official style guide - that means it's definitely worth the hassle to make a rule for it too. Do you agree?

I 90% agree. 🙂 In cases where implementing an ESLint rule would be especially labor-intensive, I think it also makes sense to take into account how common violations are (how much time are we actually saving users?). If the value for the effort is relatively low, we might want to make it a lower priority.

I'll create another label Style Guide candidate for marking propositions that we should consider adding to the style guide first.

Good idea. And I like that flow: add to the style guide first, when appropriate, then create an ESLint rule. Creating the more detailed explanation in the style guide will give us a chance to fully think through the problem and all possible acceptable styles, so that creating the ESLint rule will be easier.

[michalsnik]

I'm glad we're on the same page here ✋

[michalsnik]

@chrisvfritz I finished adding rules' propositions regarding current state of the Style Guide:

• #245
• #246
• #247
• #248
• #249
• #250
• #251
• #252
• #253
• #254
• #255
• #256
• #257

Take a look and give a thumbs up if you also agree on them :)

I also cleared out all issues, and it looks like we have only 5 rules propositions that are not present in style guide (mostly because they're too obvious).

[chrisvfritz]

@michalsnik Looks fantastic! I just went through and reviewed them all. Thank you for organizing them to make it so easy. 😄

[Maxhodges]

I get page not found. is this dead?
https://vue-style-guide-dev.surge.sh/v2/guide/style-guide.html

[Maxhodges]

also the branch cited is not found
https://github.com/vuejs/vuejs.org/blob/style-guide/src/v2/guide/style-guide.md

[chrisvfritz]

@Maxhodges That was never a live link (notice the dev in the subdomain). The published style guide is at vuejs.org/v2/style-guide.

[michalsnik]

Now that we have separate issues for different recommendation from style guide I think we can close this and continue discussions there. If something is worth to be added to Style Guide we're going to add badge Styleguide Candidate on the given proposition. Thank you all for your contribution :)

[guohuihot]

How can we do this?