From 61e93d8a8c07d1e5bc37b0761a5ac528613c3ae9 Mon Sep 17 00:00:00 2001 From: felipe Date: Thu, 7 Dec 2023 03:08:34 -0800 Subject: [PATCH 01/12] check for siblings and refactor checkForKey func --- lib/rules/v-if-else-key.js | 81 ++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 25 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index 0a6ec9cfe..e871c64a0 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -25,6 +25,33 @@ const casing = require('../utils/casing') * @property {VElement | null} else - The node associated with the 'v-else' directive, or null if there isn't one. */ +/** + * Checks if a given node has sibling nodes of the same type that are also conditionally rendered. + * This is used to determine if multiple instances of the same component are being conditionally + * rendered within the same parent scope. + * + * @param {VElement} node - The Vue component node to check for conditional rendering siblings. + * @param {string} componentName - The name of the component to check for sibling instances. + * @returns {boolean} True if there are sibling nodes of the same type and conditionally rendered, false otherwise. + */ +const hasConditionalRenderedSiblings = (node, componentName) => { + if (node.parent && node.parent.type === 'VElement') { + const siblings = node.parent.children.filter( + (child) => child.type === 'VElement' + ) + + return siblings.some( + (sibling) => + sibling !== node && + sibling.type === 'VElement' && + sibling.rawName === componentName && + hasConditionalDirective(sibling) + ) + } + + return false +} + /** * 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. @@ -44,35 +71,39 @@ const checkForKey = ( uniqueKey, conditionalFamilies ) => { - if (node.parent && node.parent.type === 'VElement') { + if ( + node.parent && + node.parent.type === 'VElement' && + hasConditionalRenderedSiblings(node, componentName) + ) { 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 - } + if (conditionalFamily && !utils.hasAttribute(node, 'key')) { + let needsKey = false - 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}"` - ) + if (node === conditionalFamily.if || node === conditionalFamily.else) { + needsKey = true + } else if (conditionalFamily.elseIf.includes(node)) { + needsKey = true } - }) + + if (needsKey) { + 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}"` + ) + } + }) + } + } } } From 96f8a3e8b28bcd49976f518997613cc29bafc154 Mon Sep 17 00:00:00 2001 From: felipe Date: Thu, 7 Dec 2023 03:08:50 -0800 Subject: [PATCH 02/12] add tests --- tests/lib/rules/v-if-else-key.js | 95 ++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/tests/lib/rules/v-if-else-key.js b/tests/lib/rules/v-if-else-key.js index 4aab257d0..d657454ee 100644 --- a/tests/lib/rules/v-if-else-key.js +++ b/tests/lib/rules/v-if-else-key.js @@ -127,6 +127,45 @@ tester.run('v-if-else-key', rule, { } ` + }, + { + filename: 'test.vue', + code: ` + + + ` } ], invalid: [ @@ -424,6 +463,62 @@ tester.run('v-if-else-key', rule, { line: 6 } ] + }, + { + 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: 7 + }, + { + message: + "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.", + line: 8 + } + ] } ] }) From f74541d4869fd78b8986e3f84f8a02e5ac405f90 Mon Sep 17 00:00:00 2001 From: felipe Date: Thu, 7 Dec 2023 03:31:31 -0800 Subject: [PATCH 03/12] add more tests --- tests/lib/rules/v-if-else-key.js | 45 ++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/lib/rules/v-if-else-key.js b/tests/lib/rules/v-if-else-key.js index d657454ee..90d6ca3e4 100644 --- a/tests/lib/rules/v-if-else-key.js +++ b/tests/lib/rules/v-if-else-key.js @@ -166,6 +166,51 @@ tester.run('v-if-else-key', rule, { } ` + }, + { + filename: 'test.vue', + code: ` + + + ` + }, + { + filename: 'test.vue', + code: ` + + + ` } ], invalid: [ From 0923acc43384a8665b382a56febeeed021520549 Mon Sep 17 00:00:00 2001 From: felipe Date: Fri, 8 Dec 2023 06:24:39 -0800 Subject: [PATCH 04/12] enhance tracking of component usage - focusing on repeated if directives --- lib/rules/v-if-else-key.js | 65 ++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js index e871c64a0..8be19c59d 100644 --- a/lib/rules/v-if-else-key.js +++ b/lib/rules/v-if-else-key.js @@ -220,7 +220,7 @@ module.exports = { if (node.parent && node.parent.type === 'VElement') { let conditionalFamily = conditionalFamilies.get(node.parent) - if (conditionType === 'if' && !conditionalFamily) { + if (conditionType === 'if') { conditionalFamily = createConditionalFamily(node) conditionalFamilies.set(node.parent, conditionalFamily) } @@ -253,44 +253,55 @@ module.exports = { const currentScope = componentUsageStack[componentUsageStack.length - 1] const usageInfo = currentScope.get(componentName) || { count: 0, - firstNode: null + firstNode: null, + hasIf: false } - 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 - ) + const isIfDirective = utils.getDirective(node, 'if') !== null + const isConditional = + isIfDirective || + utils.getDirective(node, 'else-if') !== null || + utils.getDirective(node, 'else') !== null - // If this is the second occurrence, also apply a fix to the first occurrence - if (usageInfo.count === 1) { - const uniqueKeyForFirstInstance = `${casing.kebabCase( - componentName - )}-1` + if (isConditional) { + if (isIfDirective && usageInfo.hasIf) { + // Reset if family already has an 'if' directive + usageInfo.count = 1 + usageInfo.firstNode = node + } else { + usageInfo.hasIf = true + if (usageInfo.count === 0) { + usageInfo.firstNode = node + } else if (usageInfo.count > 0 && usageInfo.firstNode !== node) { + const uniqueKey = `${casing.kebabCase(componentName)}-${ + usageInfo.count + 1 + }` checkForKey( - usageInfo.firstNode, + node, context, componentName, - uniqueKeyForFirstInstance, + uniqueKey, conditionalFamilies ) + + if (usageInfo.count === 1) { + const uniqueKeyForFirstInstance = `${casing.kebabCase( + componentName + )}-1` + checkForKey( + usageInfo.firstNode, + context, + componentName, + uniqueKeyForFirstInstance, + conditionalFamilies + ) + } } + usageInfo.count++ } - usageInfo.count += 1 currentScope.set(componentName, usageInfo) } + componentUsageStack.push(new Map()) pushedNodes.add(node) }, From e978c9d0e43448cb599dedf4a5168602f80c697c Mon Sep 17 00:00:00 2001 From: felipe Date: Fri, 8 Dec 2023 06:24:59 -0800 Subject: [PATCH 05/12] fix test --- tests/lib/rules/v-if-else-key.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/lib/rules/v-if-else-key.js b/tests/lib/rules/v-if-else-key.js index 90d6ca3e4..c3715b060 100644 --- a/tests/lib/rules/v-if-else-key.js +++ b/tests/lib/rules/v-if-else-key.js @@ -532,11 +532,11 @@ tester.run('v-if-else-key', rule, { output: `