Skip to content

Commit 60a36c2

Browse files
authored
[New] Add rule vue/no-use-v-if-with-v-for (#2)
1 parent abbde6f commit 60a36c2

File tree

5 files changed

+314
-0
lines changed

5 files changed

+314
-0
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ Enforce all the rules in this category, as well as all higher priority rules, wi
138138
| | [vue/no-template-key](./docs/rules/no-template-key.md) | disallow `key` attribute on `<template>` |
139139
| | [vue/no-textarea-mustache](./docs/rules/no-textarea-mustache.md) | disallow mustaches in `<textarea>` |
140140
| | [vue/no-unused-vars](./docs/rules/no-unused-vars.md) | disallow unused variable definitions of v-for directives or scope attributes |
141+
| | [vue/no-use-v-if-with-v-for](./docs/rules/no-use-v-if-with-v-for.md) | disallow use v-if on the same element as v-for. |
141142
| | [vue/require-component-is](./docs/rules/require-component-is.md) | require `v-bind:is` of `<component>` elements |
142143
| | [vue/require-render-return](./docs/rules/require-render-return.md) | enforce render function to always return value |
143144
| | [vue/require-v-for-key](./docs/rules/require-v-for-key.md) | require `v-bind:key` with `v-for` directives |

docs/rules/no-use-v-if-with-v-for.md

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
# disallow use v-if on the same element as v-for. (vue/no-use-v-if-with-v-for)
2+
3+
> Never use `v-if` on the same element as `v-for`.
4+
>
5+
> There are two common cases where this can be tempting:
6+
>
7+
> * To filter items in a list (e.g. `v-for="user in users" v-if="user.isActive"`). In these cases, replace `users` with a new computed property that returns your filtered list (e.g. `activeUsers`).
8+
>
9+
> * To avoid rendering a list if it should be hidden (e.g. `v-for="user in users" v-if="shouldShowUsers"`). In these cases, move the `v-if` to a container element (e.g. `ul`, `ol`).
10+
>
11+
> https://vuejs.org/v2/style-guide/#Avoid-v-if-with-v-for-essential
12+
13+
14+
## :book: Rule Details
15+
16+
:-1: Examples of **incorrect** code for this rule:
17+
18+
```html
19+
<TodoItem
20+
v-if="complete"
21+
v-for="todo in todos"
22+
:todo="todo"
23+
/>
24+
```
25+
26+
In this case, the `v-if` should be written on the wrapper element.
27+
28+
29+
```html
30+
<TodoItem
31+
v-for="todo in todos"
32+
v-if="todo.shown"
33+
:todo="todo"
34+
/>
35+
```
36+
37+
In this case, the `v-for` list variable should be replace with a computed property that returns your filtered list.
38+
39+
40+
:+1: Examples of **correct** code for this rule:
41+
42+
43+
```html
44+
<ul v-if="complete">
45+
<TodoItem
46+
v-for="todo in todos"
47+
:todo="todo"
48+
/>
49+
</ul>
50+
```
51+
52+
53+
54+
```html
55+
<TodoItem
56+
v-for="todo in shownTodos"
57+
:todo="todo"
58+
/>
59+
```
60+
61+
```js
62+
computed: {
63+
shownTodos() {
64+
return this.todos.filter((todo) => todo.shown)
65+
}
66+
}
67+
```
68+
69+
## :wrench: Options
70+
71+
`allowUsingIterationVar` - Enables The `v-if` directive use the reference which is to the variables which are defined by the `v-for` directives.
72+
73+
```js
74+
'vue/no-use-v-if-with-v-for': ['error', {
75+
allowUsingIterationVar: true // default: false
76+
}]
77+
```
78+
79+
:+1: Examples of additional **correct** code for this rule with sample `"allowUsingIterationVar": true` options:
80+
81+
```html
82+
<TodoItem
83+
v-if="complete"
84+
v-for="todo in todos"
85+
:todo="todo"
86+
/>
87+
```
88+
89+
:-1: Examples of additional **incorrect** code for this rule with sample `"allowUsingIterationVar": true` options:
90+
91+
```html
92+
<TodoItem
93+
v-for="todo in todos"
94+
v-if="todo.shown"
95+
:todo="todo"
96+
/>
97+
```
98+

lib/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ module.exports = {
3232
'no-template-key': require('./rules/no-template-key'),
3333
'no-textarea-mustache': require('./rules/no-textarea-mustache'),
3434
'no-unused-vars': require('./rules/no-unused-vars'),
35+
'no-use-v-if-with-v-for': require('./rules/no-use-v-if-with-v-for'),
3536
'order-in-components': require('./rules/order-in-components'),
3637
'require-component-is': require('./rules/require-component-is'),
3738
'require-default-prop': require('./rules/require-default-prop'),

lib/rules/no-use-v-if-with-v-for.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* @author Yosuke Ota
3+
*
4+
* issue https://github.com/vuejs/eslint-plugin-vue/issues/403
5+
* Style guide: https://vuejs.org/v2/style-guide/#Avoid-v-if-with-v-for-essential
6+
*
7+
* I implemented it with reference to `no-confusing-v-for-v-if`
8+
*/
9+
'use strict'
10+
11+
// ------------------------------------------------------------------------------
12+
// Requirements
13+
// ------------------------------------------------------------------------------
14+
15+
const utils = require('../utils')
16+
17+
// ------------------------------------------------------------------------------
18+
// Helpers
19+
// ------------------------------------------------------------------------------
20+
21+
/**
22+
* Check whether the given `v-if` node is using the variable which is defined by the `v-for` directive.
23+
* @param {ASTNode} vIf The `v-if` attribute node to check.
24+
* @returns {boolean} `true` if the `v-if` is using the variable which is defined by the `v-for` directive.
25+
*/
26+
function isUsingIterationVar (vIf) {
27+
const element = vIf.parent.parent
28+
return vIf.value.references.some(reference =>
29+
element.variables.some(variable =>
30+
variable.id.name === reference.id.name &&
31+
variable.kind === 'v-for'
32+
)
33+
)
34+
}
35+
36+
// ------------------------------------------------------------------------------
37+
// Rule Definition
38+
// ------------------------------------------------------------------------------
39+
40+
module.exports = {
41+
meta: {
42+
docs: {
43+
description: 'disallow use v-if on the same element as v-for.',
44+
category: undefined,
45+
url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v4.2.2/docs/rules/no-use-v-if-with-v-for.md'
46+
},
47+
fixable: null,
48+
schema: [{
49+
type: 'object',
50+
properties: {
51+
allowUsingIterationVar: {
52+
type: 'boolean'
53+
}
54+
}
55+
}]
56+
},
57+
58+
create (context) {
59+
const options = context.options[0] || {}
60+
const allowUsingIterationVar = options.allowUsingIterationVar === true // default false
61+
return utils.defineTemplateBodyVisitor(context, {
62+
"VAttribute[directive=true][key.name='if']" (node) {
63+
const element = node.parent.parent
64+
65+
if (utils.hasDirective(element, 'for')) {
66+
if (isUsingIterationVar(node)) {
67+
if (!allowUsingIterationVar) {
68+
context.report({
69+
node,
70+
loc: node.loc,
71+
message: "The 'v-for' list variable should be replace with a new computed property that returns your filtered list by this 'v-if' condition."
72+
})
73+
}
74+
} else {
75+
context.report({
76+
node,
77+
loc: node.loc,
78+
message: "This 'v-if' should be moved to the wrapper element."
79+
})
80+
}
81+
}
82+
}
83+
})
84+
}
85+
}
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/**
2+
* @author Yosuke Ota
3+
*/
4+
'use strict'
5+
6+
// ------------------------------------------------------------------------------
7+
// Requirements
8+
// ------------------------------------------------------------------------------
9+
10+
const RuleTester = require('eslint').RuleTester
11+
const rule = require('../../../lib/rules/no-use-v-if-with-v-for')
12+
13+
// ------------------------------------------------------------------------------
14+
// Tests
15+
// ------------------------------------------------------------------------------
16+
17+
const tester = new RuleTester({
18+
parser: 'vue-eslint-parser',
19+
parserOptions: { ecmaVersion: 2015 }
20+
})
21+
22+
tester.run('no-use-v-if-with-v-for', rule, {
23+
valid: [
24+
{
25+
filename: 'test.vue',
26+
code: ''
27+
},
28+
{
29+
filename: 'test.vue',
30+
code: '<template><div><div v-for="x in list" v-if="x"></div></div></template>',
31+
options: [{ allowUsingIterationVar: true }]
32+
},
33+
{
34+
filename: 'test.vue',
35+
code: '<template><div><div v-for="x in list" v-if="x.foo"></div></div></template>',
36+
options: [{ allowUsingIterationVar: true }]
37+
},
38+
{
39+
filename: 'test.vue',
40+
code: '<template><div><div v-for="(x,i) in list" v-if="i%2==0"></div></div></template>',
41+
options: [{ allowUsingIterationVar: true }]
42+
},
43+
{
44+
filename: 'test.vue',
45+
code: '<template><div v-if="shown"><div v-for="(x,i) in list"></div></div></template>'
46+
},
47+
{
48+
filename: 'test.vue',
49+
code: `
50+
<template>
51+
<ul>
52+
<li
53+
v-for="user in activeUsers"
54+
:key="user.id"
55+
>
56+
{{ user.name }}
57+
<li>
58+
</ul>
59+
</template>
60+
`
61+
},
62+
{
63+
filename: 'test.vue',
64+
code: `
65+
<template>
66+
<ul v-if="shouldShowUsers">
67+
<li
68+
v-for="user in users"
69+
:key="user.id"
70+
>
71+
{{ user.name }}
72+
<li>
73+
</ul>
74+
</template>
75+
`
76+
}
77+
],
78+
invalid: [
79+
{
80+
filename: 'test.vue',
81+
code: '<template><div><div v-for="x in list" v-if="shown"></div></div></template>',
82+
errors: ["This 'v-if' should be moved to the wrapper element."]
83+
},
84+
{
85+
filename: 'test.vue',
86+
code: '<template><div><div v-for="x in list" v-if="list.length&gt;0"></div></div></template>',
87+
errors: ["This 'v-if' should be moved to the wrapper element."]
88+
},
89+
{
90+
filename: 'test.vue',
91+
code: '<template><div><div v-for="x in list" v-if="x.isActive"></div></div></template>',
92+
errors: ["The 'v-for' list variable should be replace with a new computed property that returns your filtered list by this 'v-if' condition."]
93+
},
94+
{
95+
filename: 'test.vue',
96+
code: `
97+
<template>
98+
<ul>
99+
<li
100+
v-for="user in users"
101+
v-if="user.isActive"
102+
:key="user.id"
103+
>
104+
{{ user.name }}
105+
<li>
106+
</ul>
107+
</template>
108+
`,
109+
errors: ["The 'v-for' list variable should be replace with a new computed property that returns your filtered list by this 'v-if' condition."]
110+
},
111+
{
112+
filename: 'test.vue',
113+
code: `
114+
<template>
115+
<ul>
116+
<li
117+
v-for="user in users"
118+
v-if="shouldShowUsers"
119+
:key="user.id"
120+
>
121+
{{ user.name }}
122+
<li>
123+
</ul>
124+
</template>
125+
`,
126+
errors: ["This 'v-if' should be moved to the wrapper element."]
127+
}
128+
]
129+
})

0 commit comments

Comments
 (0)