eslint-plugin-vue icon indicating copy to clipboard operation
eslint-plugin-vue copied to clipboard

Replace require-v-for-key with new rule just for stateful elements

Open chrisvfritz opened this issue 6 years ago • 5 comments

The Problem

A lot of users get upset when the linter yells at them for not providing a key when it doesn't create a real problem and Vue itself doesn't complain. The real intent with this rule is to maintain object constancy to avoid:

  • Losing state in non-functional components (e.g. data properties being reset, lifecycle methods being re-run unnecessarily)
  • Losing state in stateful non-component elements such as input, select, etc (e.g. value being lost if not passed as a prop)
  • Visual inconsistencies on elements wrapped by <transition> or <transition-group> elements.

Potential Solution

To make everyone happy, I'm wondering if we may want to rename require-v-for-key to something like require-v-for-key-stateful-elements, then change its behavior so that it only gives a warning on:

  • Components
  • Stateful non-component elements
  • Non-stateful elements that use v-html
  • Non-stateful elements that contain any of the above
  • Non-stateful elements whose parent element is <transition> or <transition-group>.

Thoughts?

I'd like to get some other people weighing in on this before making the change, as there may be other cases I'm not considering.

chrisvfritz avatar Jul 15 '18 21:07 chrisvfritz

I'm adding some specific messages and examples for each case here, to make building the rule easier.

Missing key with v-for on component

Message

Missing key attribute on a component using v-for. A key is required to help Vue uniquely identify each component in the list, so that it can avoid recreating components and destroying their local state.

Examples

<my-component v-for="n in 2"/>
<MyComponent v-for="n in 2"/>

Missing key with v-for on stateful element

Message

Missing key attribute on a stateful element (e.g. input, select, etc) using v-for. A key is required to help Vue uniquely identify each element in the list, so that it can avoid recreating these and destroying their state.

Examples

<input v-for="n in 2">
<select v-for="n in 2">
  ...
</select>

Missing key with v-for on element using v-html

Message

Missing key attribute on an using v-for and v-html. A key is required to help Vue uniquely identify each element in the list, so that it can avoid recreating these and potentially destroying the local state of any stateful elements (e.g. input, select, etc) that they render.

Examples

<div v-for="n in 2" v-html="n"/>
<span v-for="n in 2" v-html="'<p>hi</p>'"/>

Missing key with v-for on element with stateful content

Message

Missing key attribute on an element with stateful content. A key is required to help Vue uniquely identify each element in the list, so that it can avoid recreating its content and potentially destroying the local state of any components or stateful elements (e.g. input, select, etc) they contain.

Examples

<div v-for="n in 2">
  <my-component/>
</div>
<div v-for="n in 2">
  <input>
</div>
<div v-for="n in 2">
  <div v-html="n"/>
</div>

Non-unique key with v-for

Message

The key attribute on elements using v-for must use v-bind to reference a property defined by the v-for, so that the value of each key will be unique.

Examples

<MyComponent v-for="n in 2" key="foo"/>
<!-- Does not use any properties defined in `v-for` -->
<MyComponent v-for="n in 2" v-bind:key="foo"/>
<!-- Does not use any properties defined in `v-for` -->
<MyComponent v-for="n in 2" :key="foo"/>

Notes

This would require us to be able to access the names of any properties (e.g. n in the examples above) and ensure that it is used within the value of the key, if the key if using v-bind.

Missing key with v-for on transitioned element

Message

Missing key attribute on an element with a parent of <transition> or <transition-group>. A key is required to help Vue uniquely identify each element in the list, so that it can maintain visual object constancy.

Examples

<transition>
  <div v-for="n in 2"/>
</transition>
<transition-group>
  <div v-for="n in 2"/>
</transition-group>

Notes

This one isn't really related to v-for specifically, as it also applies to elements with v-if/v-else-if/v-else that share a tag name. So maybe it should be in a separate rule?

chrisvfritz avatar Jul 18 '18 17:07 chrisvfritz

This is how all proposal should look like! Really good piece of work @chrisvfritz 🥇

I agree with everything, but one case: Non-unique key with v-for - I mean it's all good, but I'm thinking whether this single case shouldn't be in a separate rule, as it's kind of independent from what's being proposed in other examples, and one rule should do one thing in eslint.

So the way I see it is by having two separate rules:

  • require-v-for-key (we need to update current one)
  • unique-v-for-key (new)

We should also take cases with <template> and <slot> into consideration, am I right?

<template v-for="n in 2">
  <my-component/> <!-- missing key -->
</template>
<template v-for="n in 2">
  <input> <!-- missing key -->
</template>
<template v-for="n in 2">
  <div v-html="n" /> <!-- missing key -->
</template>

michalsnik avatar Jul 18 '18 22:07 michalsnik

This is how all proposal should look like! Really good piece of work @chrisvfritz 🥇

:blush: 🙈

I agree with everything, but one case: Non-unique key with v-for - I mean it's all good, but I'm thinking whether this single case shouldn't be in a separate rule, as it's kind of independent from what's being proposed in other examples, and one rule should do one thing in eslint.

That sounds good to me. 🙂

We should also take cases with

Yes, absolutely!

chrisvfritz avatar Jul 19 '18 07:07 chrisvfritz

Hello people, any progress on this? I could probably take a shot at it, with a bit of guidance from a maintainer.

Also, as a suggestion, can we also add a :key="index" check to all the scenarios identified by @chrisvfritz ? In other words, emit a warning if either there's no key, or if key is set to the v-for index (of which I've encountered lots of cases while helping people on the Vue official chat). I'm not sure if this should be part of the same rule (although, implementation wise, the check and the warning message would be very similar), any thoughts?

lbogdan avatar Jan 06 '19 17:01 lbogdan

@michalsnik @chrisvfritz Ping?

lbogdan avatar Jan 22 '19 18:01 lbogdan