Skip to content

Require key for conditionally rendered repeated components #2280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
76ae5c7
create rule
felipemelendez Aug 28, 2023
11769be
create tests
felipemelendez Aug 28, 2023
7899246
add docs
felipemelendez Aug 28, 2023
a02a577
fix spacing
felipemelendez Aug 28, 2023
b7a7cbb
slots are abstract outlets and can possibly expand into multiple elem…
felipemelendez Aug 28, 2023
17323fa
add test for slot exclusion
felipemelendez Aug 28, 2023
a5d7879
use more efficient Set rather than Array
felipemelendez Aug 28, 2023
308e2a1
change vue 3 to vue 2
felipemelendez Aug 28, 2023
0daeb91
target custom components
felipemelendez Oct 25, 2023
c4871c6
rename
felipemelendez Oct 25, 2023
4aac97a
Merge branch 'master' into require-key-for-conditionally-rendered-rep…
felipemelendez Oct 25, 2023
88d57d4
proper name updating
felipemelendez Oct 25, 2023
4515954
Merge remote-tracking branch 'origin/require-key-for-conditionally-re…
felipemelendez Oct 25, 2023
c659339
removed references to html container elements
felipemelendez Oct 27, 2023
c89efd8
added test
felipemelendez Oct 27, 2023
1f81ded
Merge branch 'master' into require-key-for-conditionally-rendered-rep…
felipemelendez Oct 27, 2023
d1b5039
add valid test case
felipemelendez Oct 27, 2023
fdd59a5
add Related rule link to require-v-for-key and vice-versa
felipemelendez Oct 27, 2023
92bdc2f
remove md extension from brackets
felipemelendez Oct 27, 2023
3212653
turned off formatting settings so I can submit this - changing the th…
felipemelendez Oct 27, 2023
1206bbd
make if attribute required and finetune docstrings
felipemelendez Nov 1, 2023
8da18db
fix type
felipemelendez Nov 20, 2023
a8709d7
fix condition
felipemelendez Nov 20, 2023
f039fb7
use double quotes
felipemelendez Nov 20, 2023
2ff3a5f
fix conditional family completeness logic and add test
felipemelendez Nov 20, 2023
608b4b9
reform helper function - exists only to ease code readability
felipemelendez Nov 21, 2023
42500ce
inline logic for conditional family formation
felipemelendez Nov 21, 2023
51cef47
match test file name with rule name
felipemelendez Nov 29, 2023
e6d5c92
move conditionalFamilies map into create function
felipemelendez Nov 29, 2023
ceab4c3
Merge branch 'master' into require-key-for-conditionally-rendered-rep…
felipemelendez Nov 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ For example:
| [vue/sort-keys](./sort-keys.md) | enforce sort-keys in a manner that is compatible with order-in-components | | :hammer: |
| [vue/static-class-names-order](./static-class-names-order.md) | enforce static class names order | :wrench: | :hammer: |
| [vue/v-for-delimiter-style](./v-for-delimiter-style.md) | enforce `v-for` directive's delimiter style | :wrench: | :lipstick: |
| [vue/v-if-else-key](./v-if-else-key.md) | require key attribute for conditionally rendered repeated components | :wrench: | :warning: |
| [vue/v-on-handler-style](./v-on-handler-style.md) | enforce writing style for handlers in `v-on` directives | :wrench: | :hammer: |
| [vue/valid-define-options](./valid-define-options.md) | enforce valid `defineOptions` compiler macro | | :warning: |

Expand Down
10 changes: 5 additions & 5 deletions docs/rules/require-v-for-key.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ title: vue/require-v-for-key
description: require `v-bind:key` with `v-for` directives
since: v3.0.0
---

# vue/require-v-for-key

> require `v-bind:key` with `v-for` directives
Expand All @@ -20,12 +21,9 @@ This rule reports the elements which have `v-for` and do not have `v-bind:key` w
```vue
<template>
<!-- ✓ GOOD -->
<div
v-for="todo in todos"
:key="todo.id"
/>
<div v-for="todo in todos" :key="todo.id" />
<!-- ✗ BAD -->
<div v-for="todo in todos"/>
<div v-for="todo in todos" />
</template>
```

Expand All @@ -43,8 +41,10 @@ Nothing.
## :couple: Related Rules

- [vue/valid-v-for]
- [vue/v-if-else-key]

[vue/valid-v-for]: ./valid-v-for.md
[vue/v-if-else-key]: ./v-if-else-key.md

## :books: Further Reading

Expand Down
56 changes: 56 additions & 0 deletions docs/rules/v-if-else-key.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
pageClass: rule-details
sidebarDepth: 0
title: vue/v-if-else-key
description: require key attribute for conditionally rendered repeated components
---

# vue/v-if-else-key

> require key attribute for conditionally rendered repeated components

- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> ***This rule has not been released yet.*** </badge>
- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

## :book: Rule Details

This rule checks for components that are both repeated and conditionally rendered within the same scope. If such a component is found, the rule then checks for the presence of a 'key' directive. If the 'key' directive is missing, the rule issues a warning and offers a fix.

This rule is not required in Vue 3, as the key is automatically assigned to the elements.

<eslint-code-block fix :rules="{'vue/v-if-else-key': ['error']}">

```vue
<template>
<!-- ✓ GOOD -->
<my-component v-if="condition1" :key="one" />
<my-component v-else-if="condition2" :key="two" />
<my-component v-else :key="three" />

<!-- ✗ BAD -->
<my-component v-if="condition1" />
<my-component v-else-if="condition2" />
<my-component v-else />
</template>
```

</eslint-code-block>

## :wrench: Options

Nothing.

## :couple: Related Rules

- [vue/require-v-for-key]

[vue/require-v-for-key]: ./require-v-for-key.md

## :books: Further Reading

- [Guide (for v2) - v-if without key](https://v2.vuejs.org/v2/style-guide/#v-if-v-else-if-v-else-without-key-use-with-caution)

## :mag: Implementation

- [Rule source](https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/v-if-else-key.js)
- [Test source](https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/v-if-else-key.js)
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ module.exports = {
'use-v-on-exact': require('./rules/use-v-on-exact'),
'v-bind-style': require('./rules/v-bind-style'),
'v-for-delimiter-style': require('./rules/v-for-delimiter-style'),
'v-if-else-key': require('./rules/v-if-else-key'),
'v-on-event-hyphenation': require('./rules/v-on-event-hyphenation'),
'v-on-function-call': require('./rules/v-on-function-call'),
'v-on-handler-style': require('./rules/v-on-handler-style'),
Expand Down
285 changes: 285 additions & 0 deletions lib/rules/v-if-else-key.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,285 @@
/**
* @author Felipe Melendez
* See LICENSE file in root directory for full license.
*/
'use strict'

// =============================================================================
// Requirements
// =============================================================================

const utils = require('../utils')
const casing = require('../utils/casing')

// =============================================================================
// Rule Helpers
// =============================================================================

/**
* A conditional family is made up of a group of repeated components that are conditionally rendered
* using v-if, v-else-if, and v-else.
*
* @typedef {Object} ConditionalFamily
* @property {VElement} if - The node associated with the 'v-if' directive.
* @property {VElement[]} elseIf - An array of nodes associated with 'v-else-if' directives.
* @property {VElement | null} else - The node associated with the 'v-else' directive, or null if there isn't one.
*/

/**
* Checks for the presence of a 'key' attribute in the given node. If the 'key' attribute is missing
* and the node is part of a conditional family a report is generated.
* The fix proposed adds a unique key based on the component's name and count,
* following the format '${kebabCase(componentName)}-${componentCount}', e.g., 'some-component-2'.
*
* @param {VElement} node - The Vue component node to check for a 'key' attribute.
* @param {RuleContext} context - The rule's context object, used for reporting.
* @param {string} componentName - Name of the component.
* @param {string} uniqueKey - A unique key for the repeated component, used for the fix.
* @param {Map<VElement, ConditionalFamily>} conditionalFamilies - Map of conditionally rendered components and their respective conditional directives.
*/
const checkForKey = (
node,
context,
componentName,
uniqueKey,
conditionalFamilies
) => {
if (node.parent && node.parent.type === 'VElement') {
const conditionalFamily = conditionalFamilies.get(node.parent)

if (
conditionalFamily &&
(utils.hasDirective(node, 'bind', 'key') ||
utils.hasAttribute(node, 'key') ||
!hasConditionalDirective(node) ||
!(conditionalFamily.else || conditionalFamily.elseIf.length > 0))
) {
return
}

context.report({
node: node.startTag,
loc: node.startTag.loc,
messageId: 'requireKey',
data: {
componentName
},
fix(fixer) {
const afterComponentNamePosition =
node.startTag.range[0] + componentName.length + 1
return fixer.insertTextBeforeRange(
[afterComponentNamePosition, afterComponentNamePosition],
` key="${uniqueKey}"`
)
}
})
}
}

/**
* Checks for the presence of conditional directives in the given node.
*
* @param {VElement} node - The node to check for conditional directives.
* @returns {boolean} Returns true if a conditional directive is found in the node or its parents,
* false otherwise.
*/
const hasConditionalDirective = (node) =>
utils.hasDirective(node, 'if') ||
utils.hasDirective(node, 'else-if') ||
utils.hasDirective(node, 'else')

// =============================================================================
// Rule Definition
// =============================================================================

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: 'problem',
docs: {
description:
'require key attribute for conditionally rendered repeated components',
categories: null,
recommended: false,
url: 'https://eslint.vuejs.org/rules/v-if-else-key.html'
},
fixable: 'code',
schema: [],
messages: {
requireKey:
"Conditionally rendered repeated component '{{componentName}}' expected to have a 'key' attribute."
}
},
/**
* Creates and returns a rule object which checks usage of repeated components. If a component
* is used more than once, it checks for the presence of a key.
*
* @param {RuleContext} context - The context object.
* @returns {Object} A dictionary of functions to be called on traversal of the template body by
* the eslint parser.
*/
create(context) {
/**
* Map to store conditionally rendered components and their respective conditional directives.
*
* @type {Map<VElement, ConditionalFamily>}
*/
const conditionalFamilies = new Map()

/**
* Array of Maps to keep track of components and their usage counts along with the first
* node instance. Each Map represents a different scope level, and maps a component name to
* an object containing the count and a reference to the first node.
*/
/** @type {Map<string, { count: number; firstNode: any }>[]} */
const componentUsageStack = [new Map()]

/**
* Checks if a given node represents a custom component without any conditional directives.
*
* @param {VElement} node - The AST node to check.
* @returns {boolean} True if the node represents a custom component without any conditional directives, false otherwise.
*/
const isCustomComponentWithoutCondition = (node) =>
node.type === 'VElement' &&
utils.isCustomComponent(node) &&
!hasConditionalDirective(node)

/** Set of built-in Vue components that are exempt from the rule. */
/** @type {Set<string>} */
const exemptTags = new Set(['component', 'slot', 'template'])

/** Set to keep track of nodes we've pushed to the stack. */
/** @type {Set<any>} */
const pushedNodes = new Set()

/**
* Creates and returns an object representing a conditional family.
*
* @param {VElement} ifNode - The VElement associated with the 'v-if' directive.
* @returns {ConditionalFamily}
*/
const createConditionalFamily = (ifNode) => ({
if: ifNode,
elseIf: [],
else: null
})

return utils.defineTemplateBodyVisitor(context, {
/**
* Callback to be executed when a Vue element is traversed. This function checks if the
* element is a component, increments the usage count of the component in the
* current scope, and checks for the key directive if the component is repeated.
*
* @param {VElement} node - The traversed Vue element.
*/
VElement(node) {
if (exemptTags.has(node.rawName)) {
return
}

const condition =
utils.getDirective(node, 'if') ||
utils.getDirective(node, 'else-if') ||
utils.getDirective(node, 'else')

if (condition) {
const conditionType = condition.key.name.name

if (node.parent && node.parent.type === 'VElement') {
let conditionalFamily = conditionalFamilies.get(node.parent)

if (conditionType === 'if' && !conditionalFamily) {
conditionalFamily = createConditionalFamily(node)
conditionalFamilies.set(node.parent, conditionalFamily)
}

if (conditionalFamily) {
switch (conditionType) {
case 'else-if': {
conditionalFamily.elseIf.push(node)
break
}
case 'else': {
conditionalFamily.else = node
break
}
}
}
}
}

if (isCustomComponentWithoutCondition(node)) {
componentUsageStack.push(new Map())
return
}

if (!utils.isCustomComponent(node)) {
return
}

const componentName = node.rawName
const currentScope = componentUsageStack[componentUsageStack.length - 1]
const usageInfo = currentScope.get(componentName) || {
count: 0,
firstNode: null
}

if (hasConditionalDirective(node)) {
// Store the first node if this is the first occurrence
if (usageInfo.count === 0) {
usageInfo.firstNode = node
}

if (usageInfo.count > 0) {
const uniqueKey = `${casing.kebabCase(componentName)}-${
usageInfo.count + 1
}`
checkForKey(
node,
context,
componentName,
uniqueKey,
conditionalFamilies
)

// If this is the second occurrence, also apply a fix to the first occurrence
if (usageInfo.count === 1) {
const uniqueKeyForFirstInstance = `${casing.kebabCase(
componentName
)}-1`
checkForKey(
usageInfo.firstNode,
context,
componentName,
uniqueKeyForFirstInstance,
conditionalFamilies
)
}
}
usageInfo.count += 1
currentScope.set(componentName, usageInfo)
}
componentUsageStack.push(new Map())
pushedNodes.add(node)
},

'VElement:exit'(node) {
if (exemptTags.has(node.rawName)) {
return
}
if (isCustomComponentWithoutCondition(node)) {
componentUsageStack.pop()
return
}
if (!utils.isCustomComponent(node)) {
return
}
if (pushedNodes.has(node)) {
componentUsageStack.pop()
pushedNodes.delete(node)
}
}
})
}
}
Loading