Fix siblings and interleaving issues reported in #2342 (#2348)

vuejs · Jul 13, 2024 · 79e13d1 · 79e13d1
1 parent 0c6f87a
commit 79e13d1
Show file tree

Hide file tree

Showing 2 changed files with 199 additions and 15 deletions.
diff --git a/lib/rules/v-if-else-key.js b/lib/rules/v-if-else-key.js
@@ -25,6 +25,28 @@ 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') {
+    return false
+  }
+  return node.parent.children.some(
+    (sibling) =>
+      sibling !== node &&
+      sibling.type === 'VElement' &&
+      sibling.rawName === componentName &&
+      hasConditionalDirective(sibling)
+  )
+}
+
 /**
  * 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,26 +66,31 @@ const checkForKey = (
   uniqueKey,
   conditionalFamilies
 ) => {
-  if (node.parent && node.parent.type === 'VElement') {
-    const conditionalFamily = conditionalFamilies.get(node.parent)
+  if (
+    !node.parent ||
+    node.parent.type !== 'VElement' ||
+    !hasConditionalRenderedSiblings(node, componentName)
+  ) {
+    return
+  }
 
-    if (
-      conditionalFamily &&
-      (utils.hasDirective(node, 'bind', 'key') ||
-        utils.hasAttribute(node, 'key') ||
-        !hasConditionalDirective(node) ||
-        !(conditionalFamily.else || conditionalFamily.elseIf.length > 0))
-    ) {
-      return
-    }
+  const conditionalFamily = conditionalFamilies.get(node.parent)
+
+  if (!conditionalFamily || utils.hasAttribute(node, 'key')) {
+    return
+  }
+
+  const needsKey =
+    conditionalFamily.if === node ||
+    conditionalFamily.else === node ||
+    conditionalFamily.elseIf.includes(node)
 
+  if (needsKey) {
     context.report({
       node: node.startTag,
       loc: node.startTag.loc,
       messageId: 'requireKey',
-      data: {
-        componentName
-      },
+      data: { componentName },
       fix(fixer) {
         const afterComponentNamePosition =
           node.startTag.range[0] + componentName.length + 1
@@ -190,13 +217,18 @@ module.exports = {
           if (node.parent && node.parent.type === 'VElement') {
             let conditionalFamily = conditionalFamilies.get(node.parent)
 
-            if (conditionType === 'if' && !conditionalFamily) {
+            if (!conditionalFamily) {
               conditionalFamily = createConditionalFamily(node)
               conditionalFamilies.set(node.parent, conditionalFamily)
             }
 
             if (conditionalFamily) {
               switch (conditionType) {
+                case 'if': {
+                  conditionalFamily = createConditionalFamily(node)
+                  conditionalFamilies.set(node.parent, conditionalFamily)
+                  break
+                }
                 case 'else-if': {
                   conditionalFamily.elseIf.push(node)
                   break

diff --git a/tests/lib/rules/v-if-else-key.js b/tests/lib/rules/v-if-else-key.js
@@ -127,6 +127,90 @@ tester.run('v-if-else-key', rule, {
         }
         </script>
         `
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <div>
+            <div>
+              <ComponentA v-if="foo" /> 
+              <ComponentB v-else-if="bar" /> 
+            </div>
+            <div>
+              <div>
+                <ComponentA v-if="foo" />
+                <ComponentB v-else-if="baz" /> 
+              </div>
+              <div>
+                <ComponentA v-if="foo" />
+                <ComponentB v-else-if="baz" />
+              </div>
+            </div>
+            <div>
+              <ComponentA v-if="foo" />
+              <ComponentB v-else /> 
+            </div>
+            <div>
+              <div v-if="foo" />
+              <ComponentB v-else />
+            </div>
+          </div>
+        </template>
+        <script>
+        export default {
+          components: {
+            ComponentA,
+            ComponentB
+          }
+        }
+        </script>
+      `
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <div>
+            <div>
+              <CustomComponent v-if="foo" />
+              <div v-else />
+            </div>
+      
+            <CustomComponent v-if="bar" />
+          </div>
+        </template>
+        <script>
+        export default {
+            components: {
+                CustomComponent
+            }
+        }
+        </script>
+        `
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <tile>
+            <template v-if="state === 'foo'">
+                <ComponentA>…</ComponentA>
+                <ComponentB>…</ComponentB>
+            </template>
+            <ComponentA v-else-if="state === 'bar'" key="a">…</ComponentA>
+            <ComponentA v-else-if="state === 'bar'" key="b">…</ComponentA>
+          </tile>
+        </template>
+        <script>
+        export default {
+            components: {
+                ComponentA,
+                ComponentB
+            }
+        }
+        </script>
+        `
     }
   ],
   invalid: [
@@ -424,6 +508,74 @@ tester.run('v-if-else-key', rule, {
           line: 6
         }
       ]
+    },
+    {
+      filename: 'test.vue',
+      code: `
+        <template>
+          <div>
+            <ComponentA v-if="foo" />
+            <ComponentA v-else />
+
+            <ComponentA v-if="bar" />
+            <ComponentA v-else-if="baz" />
+            <ComponentA v-else />
+          </div>
+        </template>
+        <script>
+        export default {
+          components: {
+            ComponentA
+          }
+        }
+        </script>
+        `,
+      output: `
+        <template>
+          <div>
+            <ComponentA key="component-a-1" v-if="foo" />
+            <ComponentA key="component-a-2" v-else />
+
+            <ComponentA key="component-a-3" v-if="bar" />
+            <ComponentA key="component-a-4" v-else-if="baz" />
+            <ComponentA key="component-a-5" v-else />
+          </div>
+        </template>
+        <script>
+        export default {
+          components: {
+            ComponentA
+          }
+        }
+        </script>
+        `,
+      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
+        },
+        {
+          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
+        },
+        {
+          message:
+            "Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.",
+          line: 9
+        }
+      ]
     }
   ]
 })