Skip to content
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

Fix siblings and interleaving issues reported in #2342 #2348

Merged
62 changes: 47 additions & 15 deletions lib/rules/v-if-else-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -44,26 +66,31 @@ const checkForKey = (
uniqueKey,
conditionalFamilies
) => {
if (node.parent && node.parent.type === 'VElement') {
const conditionalFamily = conditionalFamilies.get(node.parent)
if (
FloEdelmann marked this conversation as resolved.
Show resolved Hide resolved
!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
Expand Down Expand Up @@ -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
Expand Down
152 changes: 152 additions & 0 deletions tests/lib/rules/v-if-else-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down Expand Up @@ -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
}
]
}
]
})
Loading