From 76ae5c704b71db05a760b01566cdb871aa09f091 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 28 Aug 2023 07:53:38 -0700 Subject: [PATCH 01/26] create rule --- ...ditionally-rendered-repeated-components.js | 365 ++++++++++++++++++ 1 file changed, 365 insertions(+) create mode 100644 lib/rules/require-key-for-conditionally-rendered-repeated-components.js diff --git a/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/lib/rules/require-key-for-conditionally-rendered-repeated-components.js new file mode 100644 index 000000000..f7f33a8ea --- /dev/null +++ b/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -0,0 +1,365 @@ +/** + * @author Felipe Melendez + * See LICENSE file in root directory for full license. + */ +'use strict' + +// ============================================================================= +// Requirements +// ============================================================================= + +const utils = require('../utils') +const casing = require('../utils/casing') + +// ============================================================================= +// Constants and Variables +// ============================================================================= + +/** + * Curated list of standard HTML elements that serve as containers. + * We're not using an existing library util because we are only concerned about HTML elements that + * have the capacity to wrap other components. + * + * @type {Array} + */ +const htmlContainerElements = [ + // Structural elements + 'div', + 'section', + 'article', + 'nav', + 'aside', + 'header', + 'footer', + 'main', + + // Interactive elements + 'details', + 'menu', + + // Table related elements + 'table', + 'tr', + 'td', + 'th', + + // List related elements + 'ul', + 'ol', + 'li', + 'dl', + 'dt', + 'dd', + + // Text content or related elements + 'address', + 'figure', + 'figcaption', + 'details', + 'summary', + 'dialog', + + // Form and related elements + 'form', + 'fieldset', + 'label', + 'output', + 'optgroup', + 'select', + 'textarea' +] + +/** + * Set of HTML and Vue tags that can act as containers + * + * @type {Set} + * @constant + */ +const CONTAINER_ELEMENTS = new Set(htmlContainerElements) + +/** + * Map to store conditionally rendered components and their respective conditional directives. + * + * A conditional family is a group of components that are conditionally rendered using v-if, v-else-if, and v-else. + * This data structure helps track and manage the relation of such components with one another. + * + * This map links a parent node to its associated conditional family, representing + * the relationship between different parts of a Vue conditional rendering chain + * (v-if, v-else-if, v-else). Each entry in the map associates a parent node + * (representing the scope of the conditional family) with an object that tracks + * the nodes for each part of the conditional chain: + * + * - 'if': Represents the node associated with the 'v-if' directive. + * - 'elseIf': An array representing nodes associated with any 'v-else-if' directives. + * - 'else': Represents the node associated with the 'v-else' directive. + * + * This structure helps in understanding the relationship between conditional directives + * within a given scope and aids in linting scenarios where these relationships matter. + * + * @type {Map} + */ +const conditionalFamilies = new Map() + +// ============================================================================= +// Rule Helpers +// ============================================================================= + +/** + * Checks for the presence of a 'key' attribute in the given Vue component node. If the + * 'key' attribute is missing and the node is part of a conditional family (like v-if, v-else-if, or v-else), + * 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. + */ +const checkForKey = (node, context, componentName, uniqueKey) => { + 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) || + !isConditionalFamilyComplete(conditionalFamily)) + ) { + 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 such as 'v-if', 'v-else-if', or 'v-else' in the + * given Vue component 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') + +/** + * Checks whether a conditional family (comprising of 'if', 'else-if', and 'else' conditions) is complete. + * A conditional family is considered complete if: + * 1. An 'if' condition is present. + * 2. All elements of the family (if, else, else-if) share the same rawName. + * 3. There exists either an 'else' condition or at least one 'else-if' condition. + * + * @param {{ if: VElement | null, elseIf: VElement[], else: VElement | null }} family - The conditional family object with 'if', 'else-if', and 'else' properties. + * @returns {boolean} True if the conditional family is complete based on the above criteria, false otherwise. + */ +const isConditionalFamilyComplete = (family) => { + if (!family || !family.if) { + return false + } + const familyComponentName = family.if.rawName + if (family.else && family.else.rawName !== familyComponentName) { + return false + } + if (family.elseIf.some((node) => node.rawName !== familyComponentName)) { + return false + } + return Boolean(family.else || family.elseIf.length > 0) +} + +// ============================================================================= +// 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/require-key-for-conditionally-rendered-repeated-components.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) { + /** + * 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[]} */ + const componentUsageStack = [new Map()] + + /** Set to keep track of nodes we've pushed to the stack. */ + /** @type {Set} */ + const pushedNodes = new Set() + + /** + * Creates and returns an object representing a conditional family. + * + * @returns {{ if: VElement | null, elseIf: VElement[], else: VElement | null }} + */ + const createConditionalFamily = () => ({ + if: null, + elseIf: [], + else: null + }) + + /** + * Checks if the given node represents a container element. + * A container element is defined as a Vue AST node of type 'VElement' that: + * 1. Matches one of the predefined HTML container elements from CONTAINER_ELEMENTS. + * 2. Does NOT have a conditional directive. + * + * @param {VElement} node - The node to check. + * @returns {boolean} - True if the node is a container element, false otherwise. + */ + const isContainerElement = (node) => + node.type === 'VElement' && + CONTAINER_ELEMENTS.has(node.rawName) && + !hasConditionalDirective(node) + + return utils.defineTemplateBodyVisitor(context, { + /** + * Callback to be executed when a Vue element is traversed. This function checks if the + * Vue 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 (node.rawName === 'template') { + 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 ( + !conditionalFamily || + (conditionalFamily.if && + conditionalFamily.if.rawName !== node.rawName) + ) { + conditionalFamily = createConditionalFamily() + conditionalFamilies.set(node.parent, conditionalFamily) + } + + switch (conditionType) { + case 'if': { + conditionalFamily.if = node + break + } + case 'else-if': { + conditionalFamily.elseIf.push(node) + break + } + case 'else': { + conditionalFamily.else = node + break + } + } + } + } + + if (isContainerElement(node)) { + componentUsageStack.push(new Map()) + 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) + + // 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 + ) + } + } + usageInfo.count += 1 + currentScope.set(componentName, usageInfo) + } + componentUsageStack.push(new Map()) + pushedNodes.add(node) + }, + + 'VElement:exit'(node) { + if (node.rawName === 'template') { + return + } + if (isContainerElement(node)) { + componentUsageStack.pop() + return + } + if (pushedNodes.has(node)) { + componentUsageStack.pop() + pushedNodes.delete(node) + } + } + }) + } +} From 11769be604f0a238db67e767687371581a4dd639 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 28 Aug 2023 07:53:54 -0700 Subject: [PATCH 02/26] create tests --- ...ditionally-rendered-repeated-components.js | 261 ++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js new file mode 100644 index 000000000..518308689 --- /dev/null +++ b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -0,0 +1,261 @@ +/** + * @author Felipe Melendez + * See LICENSE file in root directory for full license. + */ +'use strict' + +const RuleTester = require('eslint').RuleTester +const rule = require('../../../lib/rules/require-key-for-conditionally-rendered-repeated-components') + +const tester = new RuleTester({ + parser: require.resolve('vue-eslint-parser'), + parserOptions: { + ecmaVersion: 2020, + sourceType: 'module' + } +}) + +tester.run('require-key-for-conditionally-rendered-repeated-components', rule, { + valid: [ + { + filename: 'test.vue', + code: ` + + + ` + }, + { + filename: 'test.vue', + code: ` + + + ` + }, + { + filename: 'test.vue', + code: ` + + + ` + } + ], + invalid: [ + { + filename: 'test.vue', + code: ` + + + `, + output: ` + + + `, + errors: [ + { + message: + "Conditionally rendered repeated component 'OuterComponent' expected to have a 'key' attribute.", + line: 4 + }, + { + message: + "Conditionally rendered repeated component 'InnerComponent' expected to have a 'key' attribute.", + line: 5 + }, + { + message: + "Conditionally rendered repeated component 'InnerComponent' expected to have a 'key' attribute.", + line: 6 + }, + { + message: + "Conditionally rendered repeated component 'OuterComponent' expected to have a 'key' attribute.", + line: 8 + } + ] + }, + { + filename: 'test.vue', + code: ` + + + `, + output: ` + + + `, + errors: [ + { + message: + "Conditionally rendered repeated component 'OuterComponent' expected to have a 'key' attribute.", + line: 4 + }, + { + message: + "Conditionally rendered repeated component 'InnerComponent' expected to have a 'key' attribute.", + line: 5 + }, + { + message: + "Conditionally rendered repeated component 'InnerComponent' expected to have a 'key' attribute.", + line: 8 + }, + { + message: + "Conditionally rendered repeated component 'OuterComponent' expected to have a 'key' attribute.", + line: 10 + } + ] + }, + { + filename: 'test.vue', + code: ` + + `, + output: ` + + `, + errors: [ + { + message: + "Conditionally rendered repeated component 'div' expected to have a 'key' attribute.", + line: 4 + }, + { + message: + "Conditionally rendered repeated component 'div' expected to have a 'key' attribute.", + line: 5 + }, + { + message: + "Conditionally rendered repeated component 'div' expected to have a 'key' attribute.", + line: 6 + } + ] + } + ] +}) From 7899246225ad3fb4b09ab5cec14da7c5a3717ad6 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 28 Aug 2023 07:54:18 -0700 Subject: [PATCH 03/26] add docs --- docs/rules/index.md | 1 + ...ditionally-rendered-repeated-components.md | 49 +++++++++++++++++++ lib/index.js | 1 + 3 files changed, 51 insertions(+) create mode 100644 docs/rules/require-key-for-conditionally-rendered-repeated-components.md diff --git a/docs/rules/index.md b/docs/rules/index.md index 224cddc7d..6d863caf7 100644 --- a/docs/rules/index.md +++ b/docs/rules/index.md @@ -267,6 +267,7 @@ For example: | [vue/require-direct-export](./require-direct-export.md) | require the component to be directly exported | | :hammer: | | [vue/require-emit-validator](./require-emit-validator.md) | require type definitions in emits | :bulb: | :hammer: | | [vue/require-expose](./require-expose.md) | require declare public properties using `expose` | :bulb: | :hammer: | +| [vue/require-key-for-conditionally-rendered-repeated-components](./require-key-for-conditionally-rendered-repeated-components.md) | require key attribute for conditionally rendered repeated components | :wrench: | :warning: | | [vue/require-macro-variable-name](./require-macro-variable-name.md) | require a certain macro variable name | :bulb: | :hammer: | | [vue/require-name-property](./require-name-property.md) | require a name property in Vue components | :bulb: | :hammer: | | [vue/require-prop-comment](./require-prop-comment.md) | require props to have a comment | | :hammer: | diff --git a/docs/rules/require-key-for-conditionally-rendered-repeated-components.md b/docs/rules/require-key-for-conditionally-rendered-repeated-components.md new file mode 100644 index 000000000..c218050f2 --- /dev/null +++ b/docs/rules/require-key-for-conditionally-rendered-repeated-components.md @@ -0,0 +1,49 @@ +--- +pageClass: rule-details +sidebarDepth: 0 +title: vue/require-key-for-conditionally-rendered-repeated-components +description: require key attribute for conditionally rendered repeated components +--- +# vue/require-key-for-conditionally-rendered-repeated-components + +> require key attribute for conditionally rendered repeated components + +- :exclamation: ***This rule has not been released yet.*** +- :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. + + + +```vue + +``` + + + +## :wrench: Options + +Nothing. + +## :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/require-key-for-conditionally-rendered-repeated-components.js) +- [Test source](https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js) diff --git a/lib/index.js b/lib/index.js index edbe34a2c..6d027c5e8 100644 --- a/lib/index.js +++ b/lib/index.js @@ -188,6 +188,7 @@ module.exports = { 'require-emit-validator': require('./rules/require-emit-validator'), 'require-explicit-emits': require('./rules/require-explicit-emits'), 'require-expose': require('./rules/require-expose'), + 'require-key-for-conditionally-rendered-repeated-components': require('./rules/require-key-for-conditionally-rendered-repeated-components'), 'require-macro-variable-name': require('./rules/require-macro-variable-name'), 'require-name-property': require('./rules/require-name-property'), 'require-prop-comment': require('./rules/require-prop-comment'), From a02a57786ed72cfcba20bf3bccd38e55b30554b2 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 28 Aug 2023 09:36:26 -0700 Subject: [PATCH 04/26] fix spacing --- ...ditionally-rendered-repeated-components.js | 224 +++++++++--------- 1 file changed, 112 insertions(+), 112 deletions(-) diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js index 518308689..3801e5cb3 100644 --- a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -22,8 +22,8 @@ tester.run('require-key-for-conditionally-rendered-repeated-components', rule, { code: ` - `, + } + + `, output: ` - - - `, + } + + `, errors: [ { message: @@ -147,55 +147,55 @@ tester.run('require-key-for-conditionally-rendered-repeated-components', rule, { { filename: 'test.vue', code: ` - - - `, + + + `, output: ` - - - `, + + + `, errors: [ { message: @@ -222,23 +222,23 @@ tester.run('require-key-for-conditionally-rendered-repeated-components', rule, { { filename: 'test.vue', code: ` - - `, + + `, output: ` - - `, + + `, errors: [ { message: From b7a7cbb6190eb10d87bd26317cddcf34deeaedc9 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 28 Aug 2023 10:26:43 -0700 Subject: [PATCH 05/26] slots are abstract outlets and can possibly expand into multiple elements so exclude --- ...for-conditionally-rendered-repeated-components.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/lib/rules/require-key-for-conditionally-rendered-repeated-components.js index f7f33a8ea..9e4eda19e 100644 --- a/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -10,6 +10,7 @@ const utils = require('../utils') const casing = require('../utils/casing') +const reservedNamesInVue3 = require('../utils/vue3-builtin-components') // ============================================================================= // Constants and Variables @@ -68,6 +69,7 @@ const htmlContainerElements = [ 'select', 'textarea' ] +const allContainerElements = [...reservedNamesInVue3, ...htmlContainerElements] /** * Set of HTML and Vue tags that can act as containers @@ -75,7 +77,7 @@ const htmlContainerElements = [ * @type {Set} * @constant */ -const CONTAINER_ELEMENTS = new Set(htmlContainerElements) +const CONTAINER_ELEMENTS = new Set(allContainerElements) /** * Map to store conditionally rendered components and their respective conditional directives. @@ -224,6 +226,10 @@ module.exports = { /** @type {Map[]} */ const componentUsageStack = [new Map()] + /** Array of built-in Vue components that are exempt from the rule. */ + /** @type {string[]} */ + const exemptTags = ['slot', 'template'] + /** Set to keep track of nodes we've pushed to the stack. */ /** @type {Set} */ const pushedNodes = new Set() @@ -262,7 +268,7 @@ module.exports = { * @param {VElement} node - The traversed Vue element. */ VElement(node) { - if (node.rawName === 'template') { + if (exemptTags.includes(node.rawName)) { return } @@ -348,7 +354,7 @@ module.exports = { }, 'VElement:exit'(node) { - if (node.rawName === 'template') { + if (exemptTags.includes(node.rawName)) { return } if (isContainerElement(node)) { From 17323fa410dd46b97bffce82417b0b5fc9b8b771 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 28 Aug 2023 10:36:07 -0700 Subject: [PATCH 06/26] add test for slot exclusion --- ...ditionally-rendered-repeated-components.js | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js index 3801e5cb3..4bf905489 100644 --- a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -21,10 +21,10 @@ tester.run('require-key-for-conditionally-rendered-repeated-components', rule, { filename: 'test.vue', code: ` + ` + }, + { + filename: 'test.vue', + code: ` + ` + }, + { + filename: 'test.vue', + code: ` + + + ` } ], invalid: [ @@ -236,44 +255,6 @@ tester.run('require-key-for-conditionally-rendered-repeated-components', rule, { line: 10 } ] - }, - { - filename: 'test.vue', - code: ` - - `, - output: ` - - `, - errors: [ - { - message: - "Conditionally rendered repeated component 'div' expected to have a 'key' attribute.", - line: 4 - }, - { - message: - "Conditionally rendered repeated component 'div' expected to have a 'key' attribute.", - line: 5 - }, - { - message: - "Conditionally rendered repeated component 'div' expected to have a 'key' attribute.", - line: 6 - } - ] } ] }) From c4871c603e145253ffb00b4b2f7ecc887fa99da1 Mon Sep 17 00:00:00 2001 From: felipe Date: Wed, 25 Oct 2023 11:11:49 -0700 Subject: [PATCH 10/26] rename --- ...epeated-components.md => v-if-else-key.md} | 25 ++++++++++--------- lib/index.js | 2 +- ...epeated-components.js => v-if-else-key.js} | 2 +- ...ditionally-rendered-repeated-components.js | 4 +-- 4 files changed, 17 insertions(+), 16 deletions(-) rename docs/rules/{require-key-for-conditionally-rendered-repeated-components.md => v-if-else-key.md} (63%) rename lib/rules/{require-key-for-conditionally-rendered-repeated-components.js => v-if-else-key.js} (99%) diff --git a/docs/rules/require-key-for-conditionally-rendered-repeated-components.md b/docs/rules/v-if-else-key.md similarity index 63% rename from docs/rules/require-key-for-conditionally-rendered-repeated-components.md rename to docs/rules/v-if-else-key.md index c218050f2..563d63db9 100644 --- a/docs/rules/require-key-for-conditionally-rendered-repeated-components.md +++ b/docs/rules/v-if-else-key.md @@ -1,14 +1,15 @@ --- pageClass: rule-details sidebarDepth: 0 -title: vue/require-key-for-conditionally-rendered-repeated-components +title: vue/v-if-else-key description: require key attribute for conditionally rendered repeated components --- -# vue/require-key-for-conditionally-rendered-repeated-components + +# vue/v-if-else-key > require key attribute for conditionally rendered repeated components -- :exclamation: ***This rule has not been released yet.*** +- :exclamation: **_This rule has not been released yet._** - :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 @@ -17,19 +18,19 @@ This rule checks for components that are both repeated and conditionally rendere This rule is not required in Vue 3, as the key is automatically assigned to the elements. - + ```vue ``` @@ -45,5 +46,5 @@ Nothing. ## :mag: Implementation -- [Rule source](https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/require-key-for-conditionally-rendered-repeated-components.js) -- [Test source](https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js) +- [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) diff --git a/lib/index.js b/lib/index.js index 6d027c5e8..f66f3a233 100644 --- a/lib/index.js +++ b/lib/index.js @@ -188,7 +188,6 @@ module.exports = { 'require-emit-validator': require('./rules/require-emit-validator'), 'require-explicit-emits': require('./rules/require-explicit-emits'), 'require-expose': require('./rules/require-expose'), - 'require-key-for-conditionally-rendered-repeated-components': require('./rules/require-key-for-conditionally-rendered-repeated-components'), 'require-macro-variable-name': require('./rules/require-macro-variable-name'), 'require-name-property': require('./rules/require-name-property'), 'require-prop-comment': require('./rules/require-prop-comment'), @@ -216,6 +215,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'), diff --git a/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/lib/rules/v-if-else-key.js similarity index 99% rename from lib/rules/require-key-for-conditionally-rendered-repeated-components.js rename to lib/rules/v-if-else-key.js index 0de4524b0..606e6b7eb 100644 --- a/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/lib/rules/v-if-else-key.js @@ -200,7 +200,7 @@ module.exports = { 'require key attribute for conditionally rendered repeated components', categories: null, recommended: false, - url: 'https://eslint.vuejs.org/rules/require-key-for-conditionally-rendered-repeated-components.html' + url: 'https://eslint.vuejs.org/rules/v-if-else-key.html' }, fixable: 'code', schema: [], diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js index fbfb44221..4474068b0 100644 --- a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -5,7 +5,7 @@ 'use strict' const RuleTester = require('eslint').RuleTester -const rule = require('../../../lib/rules/require-key-for-conditionally-rendered-repeated-components') +const rule = require('../../../lib/rules/v-if-else-key') const tester = new RuleTester({ parser: require.resolve('vue-eslint-parser'), @@ -15,7 +15,7 @@ const tester = new RuleTester({ } }) -tester.run('require-key-for-conditionally-rendered-repeated-components', rule, { +tester.run('v-if-else-key', rule, { valid: [ { filename: 'test.vue', From 88d57d487709cc2c19a97cdcda059dcf0b2da794 Mon Sep 17 00:00:00 2001 From: felipe Date: Wed, 25 Oct 2023 11:21:22 -0700 Subject: [PATCH 11/26] proper name updating --- docs/rules/index.md | 2 +- docs/rules/v-if-else-key.md | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/rules/index.md b/docs/rules/index.md index 6d863caf7..9652a44be 100644 --- a/docs/rules/index.md +++ b/docs/rules/index.md @@ -267,7 +267,6 @@ For example: | [vue/require-direct-export](./require-direct-export.md) | require the component to be directly exported | | :hammer: | | [vue/require-emit-validator](./require-emit-validator.md) | require type definitions in emits | :bulb: | :hammer: | | [vue/require-expose](./require-expose.md) | require declare public properties using `expose` | :bulb: | :hammer: | -| [vue/require-key-for-conditionally-rendered-repeated-components](./require-key-for-conditionally-rendered-repeated-components.md) | require key attribute for conditionally rendered repeated components | :wrench: | :warning: | | [vue/require-macro-variable-name](./require-macro-variable-name.md) | require a certain macro variable name | :bulb: | :hammer: | | [vue/require-name-property](./require-name-property.md) | require a name property in Vue components | :bulb: | :hammer: | | [vue/require-prop-comment](./require-prop-comment.md) | require props to have a comment | | :hammer: | @@ -277,6 +276,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: | diff --git a/docs/rules/v-if-else-key.md b/docs/rules/v-if-else-key.md index 563d63db9..883b14f6a 100644 --- a/docs/rules/v-if-else-key.md +++ b/docs/rules/v-if-else-key.md @@ -4,12 +4,11 @@ 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: **_This rule has not been released yet._** +- :exclamation: ***This rule has not been released yet.*** - :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 From c659339aca07875daedea6014c14d24d122be087 Mon Sep 17 00:00:00 2001 From: felipe Date: Fri, 27 Oct 2023 06:36:56 -0700 Subject: [PATCH 12/26] removed references to html container elements --- lib/rules/v-if-else-key.js | 95 ++++++-------------------------------- 1 file changed, 14 insertions(+), 81 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index 606e6b7eb..6585bbd3e 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -10,75 +10,11 @@ const utils = require('../utils') const casing = require('../utils/casing') -const reservedNamesInVue2 = require('../utils/vue2-builtin-components') // ============================================================================= // Constants and Variables // ============================================================================= -/** - * Curated list of standard HTML elements that serve as containers. - * We're not using an existing library util because we are only concerned about HTML elements that - * have the capacity to wrap other components. - * - * @type {Array} - */ -const htmlContainerElements = [ - // Structural elements - 'div', - 'section', - 'article', - 'nav', - 'aside', - 'header', - 'footer', - 'main', - - // Interactive elements - 'details', - 'menu', - - // Table related elements - 'table', - 'tr', - 'td', - 'th', - - // List related elements - 'ul', - 'ol', - 'li', - 'dl', - 'dt', - 'dd', - - // Text content or related elements - 'address', - 'figure', - 'figcaption', - 'details', - 'summary', - 'dialog', - - // Form and related elements - 'form', - 'fieldset', - 'label', - 'output', - 'optgroup', - 'select', - 'textarea' -] -const allContainerElements = [...reservedNamesInVue2, ...htmlContainerElements] - -/** - * Set of HTML and Vue tags that can act as containers - * - * @type {Set} - * @constant - */ -const CONTAINER_ELEMENTS = new Set(allContainerElements) - /** * Map to store conditionally rendered components and their respective conditional directives. * @@ -226,9 +162,20 @@ module.exports = { /** @type {Map[]} */ 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} */ - const exemptTags = new Set(['slot', 'template']) + const exemptTags = new Set(['component', 'slot', 'template']) /** Set to keep track of nodes we've pushed to the stack. */ /** @type {Set} */ @@ -245,20 +192,6 @@ module.exports = { else: null }) - /** - * Checks if the given node represents a container element. - * A container element is defined as a Vue AST node of type 'VElement' that: - * 1. Matches one of the predefined HTML container elements from CONTAINER_ELEMENTS. - * 2. Does NOT have a conditional directive. - * - * @param {VElement} node - The node to check. - * @returns {boolean} - True if the node is a container element, false otherwise. - */ - const isContainerElement = (node) => - node.type === 'VElement' && - CONTAINER_ELEMENTS.has(node.rawName) && - !hasConditionalDirective(node) - return utils.defineTemplateBodyVisitor(context, { /** * Callback to be executed when a Vue element is traversed. This function checks if the @@ -309,7 +242,7 @@ module.exports = { } } - if (isContainerElement(node)) { + if (isCustomComponentWithoutCondition(node)) { componentUsageStack.push(new Map()) return } @@ -361,7 +294,7 @@ module.exports = { if (exemptTags.has(node.rawName)) { return } - if (isContainerElement(node)) { + if (isCustomComponentWithoutCondition(node)) { componentUsageStack.pop() return } From c89efd80cc10ff3d67a2570155141ea283bfb83b Mon Sep 17 00:00:00 2001 From: felipe Date: Fri, 27 Oct 2023 06:37:17 -0700 Subject: [PATCH 13/26] added test --- ...ditionally-rendered-repeated-components.js | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js index 4474068b0..6de1d8aa9 100644 --- a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -255,6 +255,59 @@ tester.run('v-if-else-key', rule, { line: 10 } ] + }, + { + filename: 'test.vue', + code: ` + + + `, + output: ` + + + `, + errors: [ + { + message: + "Conditionally rendered repeated component 'InnerComponent' expected to have a 'key' attribute.", + line: 5 + }, + { + message: + "Conditionally rendered repeated component 'InnerComponent' expected to have a 'key' attribute.", + line: 6 + } + ] } ] }) From d1b5039d8213b6a3e91159b53ad0f84b85cb6f13 Mon Sep 17 00:00:00 2001 From: felipe Date: Fri, 27 Oct 2023 08:06:49 -0700 Subject: [PATCH 14/26] add valid test case --- ...nditionally-rendered-repeated-components.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js index 6de1d8aa9..b04b81688 100644 --- a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -109,6 +109,24 @@ tester.run('v-if-else-key', rule, { } ` + }, + { + filename: 'test.vue', + code: ` + + + ` } ], invalid: [ From fdd59a52b035d620455e82ccd38590495db64316 Mon Sep 17 00:00:00 2001 From: felipe Date: Fri, 27 Oct 2023 08:08:51 -0700 Subject: [PATCH 15/26] add Related rule link to require-v-for-key and vice-versa --- docs/rules/require-v-for-key.md | 10 +++++----- docs/rules/v-if-else-key.md | 9 ++++++++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/docs/rules/require-v-for-key.md b/docs/rules/require-v-for-key.md index c8f894ff5..7699e1b30 100644 --- a/docs/rules/require-v-for-key.md +++ b/docs/rules/require-v-for-key.md @@ -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 @@ -20,12 +21,9 @@ This rule reports the elements which have `v-for` and do not have `v-bind:key` w ```vue ``` @@ -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 diff --git a/docs/rules/v-if-else-key.md b/docs/rules/v-if-else-key.md index 883b14f6a..6654633ca 100644 --- a/docs/rules/v-if-else-key.md +++ b/docs/rules/v-if-else-key.md @@ -4,11 +4,12 @@ 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: ***This rule has not been released yet.*** +- :exclamation: **_This rule has not been released yet._** - :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 @@ -39,6 +40,12 @@ This rule is not required in Vue 3, as the key is automatically assigned to the Nothing. +## :couple: Related Rules + +- [vue/require-v-for-key.md] + +[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) From 92bdc2f1b43df28f23dbcf51d252e7d21f4883f5 Mon Sep 17 00:00:00 2001 From: felipe Date: Fri, 27 Oct 2023 08:20:17 -0700 Subject: [PATCH 16/26] remove md extension from brackets --- docs/rules/v-if-else-key.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/v-if-else-key.md b/docs/rules/v-if-else-key.md index 6654633ca..7ad7b271e 100644 --- a/docs/rules/v-if-else-key.md +++ b/docs/rules/v-if-else-key.md @@ -42,7 +42,7 @@ Nothing. ## :couple: Related Rules -- [vue/require-v-for-key.md] +- [vue/require-v-for-key] [vue/require-v-for-key]: ./require-v-for-key.md From 3212653d662ab4935f9f9d064e999c860298987f Mon Sep 17 00:00:00 2001 From: felipe Date: Fri, 27 Oct 2023 08:31:09 -0700 Subject: [PATCH 17/26] turned off formatting settings so I can submit this - changing the third asterisk to an underscore is a weird formatting rule --- docs/rules/v-if-else-key.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/v-if-else-key.md b/docs/rules/v-if-else-key.md index 7ad7b271e..11c8c642a 100644 --- a/docs/rules/v-if-else-key.md +++ b/docs/rules/v-if-else-key.md @@ -9,7 +9,7 @@ description: require key attribute for conditionally rendered repeated component > require key attribute for conditionally rendered repeated components -- :exclamation: **_This rule has not been released yet._** +- :exclamation: ***This rule has not been released yet.*** - :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 From 1206bbd1a8b1238c18a129c1e6d1228baa7d1067 Mon Sep 17 00:00:00 2001 From: felipe Date: Wed, 1 Nov 2023 07:52:20 -0700 Subject: [PATCH 18/26] make if attribute required and finetune docstrings --- lib/rules/v-if-else-key.js | 82 ++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 47 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index 6585bbd3e..cba2dd213 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -16,25 +16,19 @@ const casing = require('../utils/casing') // ============================================================================= /** - * Map to store conditionally rendered components and their respective conditional directives. - * - * A conditional family is a group of components that are conditionally rendered using v-if, v-else-if, and v-else. - * This data structure helps track and manage the relation of such components with one another. - * - * This map links a parent node to its associated conditional family, representing - * the relationship between different parts of a Vue conditional rendering chain - * (v-if, v-else-if, v-else). Each entry in the map associates a parent node - * (representing the scope of the conditional family) with an object that tracks - * the nodes for each part of the conditional chain: + * 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. * - * - 'if': Represents the node associated with the 'v-if' directive. - * - 'elseIf': An array representing nodes associated with any 'v-else-if' directives. - * - 'else': Represents the node associated with the 'v-else' directive. - * - * This structure helps in understanding the relationship between conditional directives - * within a given scope and aids in linting scenarios where these relationships matter. + * @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. + */ + +/** + * Map to store conditionally rendered components and their respective conditional directives. * - * @type {Map} + * @type {Map} */ const conditionalFamilies = new Map() @@ -43,10 +37,10 @@ const conditionalFamilies = new Map() // ============================================================================= /** - * Checks for the presence of a 'key' attribute in the given Vue component node. If the - * 'key' attribute is missing and the node is part of a conditional family (like v-if, v-else-if, or v-else), - * 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'. + * 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. @@ -87,8 +81,7 @@ const checkForKey = (node, context, componentName, uniqueKey) => { } /** - * Checks for the presence of conditional directives such as 'v-if', 'v-else-if', or 'v-else' in the - * given Vue component node. + * 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, @@ -100,13 +93,13 @@ const hasConditionalDirective = (node) => utils.hasDirective(node, 'else') /** - * Checks whether a conditional family (comprising of 'if', 'else-if', and 'else' conditions) is complete. + * Checks whether a conditional family is complete. * A conditional family is considered complete if: * 1. An 'if' condition is present. * 2. All elements of the family (if, else, else-if) share the same rawName. * 3. There exists either an 'else' condition or at least one 'else-if' condition. * - * @param {{ if: VElement | null, elseIf: VElement[], else: VElement | null }} family - The conditional family object with 'if', 'else-if', and 'else' properties. + * @param {{ if: VElement, elseIf: VElement[], else: VElement | null }} family - The conditional family object with 'if', 'else-if', and 'else' properties. * @returns {boolean} True if the conditional family is complete based on the above criteria, false otherwise. */ const isConditionalFamilyComplete = (family) => { @@ -184,10 +177,11 @@ module.exports = { /** * Creates and returns an object representing a conditional family. * - * @returns {{ if: VElement | null, elseIf: VElement[], else: VElement | null }} + * @param {VElement} ifNode - The VElement associated with the 'v-if' directive. + * @returns {ConditionalFamily} */ - const createConditionalFamily = () => ({ - if: null, + const createConditionalFamily = (ifNode) => ({ + if: ifNode, elseIf: [], else: null }) @@ -195,7 +189,7 @@ module.exports = { return utils.defineTemplateBodyVisitor(context, { /** * Callback to be executed when a Vue element is traversed. This function checks if the - * Vue element is a component, increments the usage count of the component in 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. @@ -216,27 +210,21 @@ module.exports = { if (node.parent && node.parent.type === 'VElement') { let conditionalFamily = conditionalFamilies.get(node.parent) - if ( - !conditionalFamily || - (conditionalFamily.if && - conditionalFamily.if.rawName !== node.rawName) - ) { - conditionalFamily = createConditionalFamily() + if (conditionType === 'if' && !conditionalFamily) { + conditionalFamily = createConditionalFamily(node) conditionalFamilies.set(node.parent, conditionalFamily) } - switch (conditionType) { - case 'if': { - conditionalFamily.if = node - break - } - case 'else-if': { - conditionalFamily.elseIf.push(node) - break - } - case 'else': { - conditionalFamily.else = node - break + if (conditionalFamily) { + switch (conditionType) { + case 'else-if': { + conditionalFamily.elseIf.push(node) + break + } + case 'else': { + conditionalFamily.else = node + break + } } } } From 8da18db699dbac5eb21bfa62aae45143493da656 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 20 Nov 2023 08:11:03 -0800 Subject: [PATCH 19/26] fix type --- lib/rules/v-if-else-key.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index cba2dd213..d56c737b6 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -99,7 +99,7 @@ const hasConditionalDirective = (node) => * 2. All elements of the family (if, else, else-if) share the same rawName. * 3. There exists either an 'else' condition or at least one 'else-if' condition. * - * @param {{ if: VElement, elseIf: VElement[], else: VElement | null }} family - The conditional family object with 'if', 'else-if', and 'else' properties. + * @param {ConditionalFamily} family - The conditional family object with 'if', 'else-if', and 'else' properties. * @returns {boolean} True if the conditional family is complete based on the above criteria, false otherwise. */ const isConditionalFamilyComplete = (family) => { From a8709d716d6c9bfd3af247ea594d38e8f68b0183 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 20 Nov 2023 08:11:30 -0800 Subject: [PATCH 20/26] fix condition --- lib/rules/v-if-else-key.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index d56c737b6..c52e44881 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -103,7 +103,7 @@ const hasConditionalDirective = (node) => * @returns {boolean} True if the conditional family is complete based on the above criteria, false otherwise. */ const isConditionalFamilyComplete = (family) => { - if (!family || !family.if) { + if (!family) { return false } const familyComponentName = family.if.rawName From f039fb77afb5236ff3cb195e76f969b8d7ce9087 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 20 Nov 2023 10:13:42 -0800 Subject: [PATCH 21/26] use double quotes --- lib/rules/v-if-else-key.js | 2 +- ...ditionally-rendered-repeated-components.js | 107 +++++++++++++----- 2 files changed, 79 insertions(+), 30 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index c52e44881..663534889 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -73,7 +73,7 @@ const checkForKey = (node, context, componentName, uniqueKey) => { node.startTag.range[0] + componentName.length + 1 return fixer.insertTextBeforeRange( [afterComponentNamePosition, afterComponentNamePosition], - ` key='${uniqueKey}'` + ` key="${uniqueKey}"` ) } }) diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js index b04b81688..4e2acb2b0 100644 --- a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -22,8 +22,8 @@ tester.run('v-if-else-key', rule, { code: ` + `, + output: ` + + + `, + errors: [ + { + message: + "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.", + line: 4 + }, + { + message: + "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.", + line: 5 + } + ] } ] }) From 2ff3a5f1df96c7b093257a161770c50ef9457df6 Mon Sep 17 00:00:00 2001 From: felipe Date: Mon, 20 Nov 2023 11:13:20 -0800 Subject: [PATCH 22/26] fix conditional family completeness logic and add test --- lib/rules/v-if-else-key.js | 11 ++--- ...ditionally-rendered-repeated-components.js | 49 +++++++++++++++++++ 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index 663534889..ea2dd6964 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -96,8 +96,7 @@ const hasConditionalDirective = (node) => * Checks whether a conditional family is complete. * A conditional family is considered complete if: * 1. An 'if' condition is present. - * 2. All elements of the family (if, else, else-if) share the same rawName. - * 3. There exists either an 'else' condition or at least one 'else-if' condition. + * 2. There exists either an 'else' condition or at least one 'else-if' condition. * * @param {ConditionalFamily} family - The conditional family object with 'if', 'else-if', and 'else' properties. * @returns {boolean} True if the conditional family is complete based on the above criteria, false otherwise. @@ -106,13 +105,11 @@ const isConditionalFamilyComplete = (family) => { if (!family) { return false } - const familyComponentName = family.if.rawName - if (family.else && family.else.rawName !== familyComponentName) { - return false - } - if (family.elseIf.some((node) => node.rawName !== familyComponentName)) { + + if (!family.if) { return false } + return Boolean(family.else || family.elseIf.length > 0) } diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js index 4e2acb2b0..4aab257d0 100644 --- a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js +++ b/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js @@ -375,6 +375,55 @@ tester.run('v-if-else-key', rule, { line: 5 } ] + }, + { + filename: 'test.vue', + code: ` + + + `, + output: ` + + + `, + errors: [ + { + message: + "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.", + line: 4 + }, + { + message: + "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.", + line: 6 + } + ] } ] }) From 608b4b93dc4dc10bd47d2321b1075b62037898d3 Mon Sep 17 00:00:00 2001 From: felipe Date: Tue, 21 Nov 2023 05:34:55 -0800 Subject: [PATCH 23/26] reform helper function - exists only to ease code readability --- lib/rules/v-if-else-key.js | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index ea2dd6964..b82f34120 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -56,7 +56,7 @@ const checkForKey = (node, context, componentName, uniqueKey) => { (utils.hasDirective(node, 'bind', 'key') || utils.hasAttribute(node, 'key') || !hasConditionalDirective(node) || - !isConditionalFamilyComplete(conditionalFamily)) + !hasFormedConditionalFamily(conditionalFamily)) ) { return } @@ -93,25 +93,14 @@ const hasConditionalDirective = (node) => utils.hasDirective(node, 'else') /** - * Checks whether a conditional family is complete. - * A conditional family is considered complete if: - * 1. An 'if' condition is present. - * 2. There exists either an 'else' condition or at least one 'else-if' condition. + * Checks whether a conditional family has formed. + * For a conditional family to form it must have an 'if' contdition and either an 'else' condition or at least one 'else-if' condition. * * @param {ConditionalFamily} family - The conditional family object with 'if', 'else-if', and 'else' properties. - * @returns {boolean} True if the conditional family is complete based on the above criteria, false otherwise. + * @returns {boolean} True if a conditional family has formed, false otherwise. */ -const isConditionalFamilyComplete = (family) => { - if (!family) { - return false - } - - if (!family.if) { - return false - } - - return Boolean(family.else || family.elseIf.length > 0) -} +const hasFormedConditionalFamily = (family) => + Boolean(family.else || family.elseIf.length > 0) // ============================================================================= // Rule Definition From 42500ce8b246e2a5f3b27ef3be87db91c3ad167a Mon Sep 17 00:00:00 2001 From: felipe Date: Tue, 21 Nov 2023 05:43:18 -0800 Subject: [PATCH 24/26] inline logic for conditional family formation --- lib/rules/v-if-else-key.js | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index b82f34120..c839daddf 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -56,7 +56,7 @@ const checkForKey = (node, context, componentName, uniqueKey) => { (utils.hasDirective(node, 'bind', 'key') || utils.hasAttribute(node, 'key') || !hasConditionalDirective(node) || - !hasFormedConditionalFamily(conditionalFamily)) + !(conditionalFamily.else || conditionalFamily.elseIf.length > 0)) ) { return } @@ -92,16 +92,6 @@ const hasConditionalDirective = (node) => utils.hasDirective(node, 'else-if') || utils.hasDirective(node, 'else') -/** - * Checks whether a conditional family has formed. - * For a conditional family to form it must have an 'if' contdition and either an 'else' condition or at least one 'else-if' condition. - * - * @param {ConditionalFamily} family - The conditional family object with 'if', 'else-if', and 'else' properties. - * @returns {boolean} True if a conditional family has formed, false otherwise. - */ -const hasFormedConditionalFamily = (family) => - Boolean(family.else || family.elseIf.length > 0) - // ============================================================================= // Rule Definition // ============================================================================= From 51cef4700b81a4b78c504d0ffc18bd814dc6e8c8 Mon Sep 17 00:00:00 2001 From: felipe Date: Wed, 29 Nov 2023 07:27:17 -0800 Subject: [PATCH 25/26] match test file name with rule name --- ...itionally-rendered-repeated-components.js => v-if-else-key.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/lib/rules/{require-key-for-conditionally-rendered-repeated-components.js => v-if-else-key.js} (100%) diff --git a/tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js b/tests/lib/rules/v-if-else-key.js similarity index 100% rename from tests/lib/rules/require-key-for-conditionally-rendered-repeated-components.js rename to tests/lib/rules/v-if-else-key.js From e6d5c92fa19af42c902f87d6adae70c254e8366d Mon Sep 17 00:00:00 2001 From: felipe Date: Wed, 29 Nov 2023 07:28:32 -0800 Subject: [PATCH 26/26] move conditionalFamilies map into create function --- lib/rules/v-if-else-key.js | 40 ++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index c839daddf..0a6ec9cfe 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -12,7 +12,7 @@ const utils = require('../utils') const casing = require('../utils/casing') // ============================================================================= -// Constants and Variables +// Rule Helpers // ============================================================================= /** @@ -25,17 +25,6 @@ const casing = require('../utils/casing') * @property {VElement | null} else - The node associated with the 'v-else' directive, or null if there isn't one. */ -/** - * Map to store conditionally rendered components and their respective conditional directives. - * - * @type {Map} - */ -const conditionalFamilies = new Map() - -// ============================================================================= -// Rule Helpers -// ============================================================================= - /** * 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. @@ -46,8 +35,15 @@ const conditionalFamilies = new Map() * @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} conditionalFamilies - Map of conditionally rendered components and their respective conditional directives. */ -const checkForKey = (node, context, componentName, uniqueKey) => { +const checkForKey = ( + node, + context, + componentName, + uniqueKey, + conditionalFamilies +) => { if (node.parent && node.parent.type === 'VElement') { const conditionalFamily = conditionalFamilies.get(node.parent) @@ -123,6 +119,13 @@ module.exports = { * the eslint parser. */ create(context) { + /** + * Map to store conditionally rendered components and their respective conditional directives. + * + * @type {Map} + */ + 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 @@ -232,7 +235,13 @@ module.exports = { const uniqueKey = `${casing.kebabCase(componentName)}-${ usageInfo.count + 1 }` - checkForKey(node, context, componentName, uniqueKey) + checkForKey( + node, + context, + componentName, + uniqueKey, + conditionalFamilies + ) // If this is the second occurrence, also apply a fix to the first occurrence if (usageInfo.count === 1) { @@ -243,7 +252,8 @@ module.exports = { usageInfo.firstNode, context, componentName, - uniqueKeyForFirstInstance + uniqueKeyForFirstInstance, + conditionalFamilies ) } }