Skip to content

Fix siblings and interleaving issues reported in #2342 #2348

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

Merged
74 changes: 49 additions & 25 deletions lib/rules/v-if-else-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,29 @@ 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 node.parent.children.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.
Expand All @@ -44,35 +67,36 @@ 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')) {
const needsKey =
conditionalFamily.if === node ||
conditionalFamily.else === node ||
conditionalFamily.elseIf.includes(node)

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 (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}"`
)
}
})
}
})
}
}
}

Expand Down
147 changes: 147 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,69 @@ 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 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: 8
},
{
message:
"Conditionally rendered repeated component 'ComponentA' expected to have a 'key' attribute.",
line: 9
}
]
}
]
})
Loading