Skip to content

Commit

Permalink
Require key for conditionally rendered repeated components (#2280)
Browse files Browse the repository at this point in the history
  • Loading branch information
felipemelendez committed Nov 30, 2023
1 parent 4eb3f50 commit 2f65e92
Show file tree
Hide file tree
Showing 6 changed files with 777 additions and 5 deletions.
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)
}
}
})
}
}

0 comments on commit 2f65e92

Please sign in to comment.