Skip to content

Commit a95dfbb

Browse files
ota-meshimichalsnik
ota-meshi
authored and
michalsnik
committed
⭐️New: Add vue/no-use-v-if-with-v-for rule (#406)
* Add Vue.extend support, add missing info about Vue.mixin check in readme * Docs: fixes wording in docs (#372) * Fix: fix script-indent to prevent removing <script> tag (fixes #367) (#374) * [Update] Make `vue/max-attributes-per-line` fixable (#380) * [Update] Make `vue/max-attributes-per-line` fixable * [fix] bug and style * [fix] Switch indent calculation method with node and attribute * [fix] don't handle indentation * [add] autofix test max-attributes-per-line.js * Update: make `vue/order-in-components` fixable (#381) * [Update] Make `vue/order-in-components` fixable This Commit makes `vue/order-in-components` fixable. In case of `The "A" property should be above the "B" property` error, autofix will move A before B * [fix] fail test at eslint@3.18.0 * [fix] If there is a possibility of a side effect, don't autofix * [fix] failed test at node v4 * [update] use Traverser * [fix] failed test eslint@3.18.0 * [fix] I used `output: null` to specify "not fix" * Fix: no-async-in-computed-properties crash (fixes #390) * Fix: valid-v-on false positive (fixes #351) * Fix: indent rules false positive (fixes #375) * [New] Add `prop-name-casing` * fix message and category * fix category * fix test message * Set category to undefined * Add more tests and fix edge case scenario * attribute order linting * updating docs, tests and category * [New] Add rule `vue/no-use-v-if-with-v-for` (#2) * [fix] `meta.docs.description` should not end with `.` consistent-docs-description * [fix] lint error caused by merging the master for conflict resolution * [fix] That `vue/attributes-order` got duplicated when merging README * [fixed] document `correct` and `incorrect` the contrary stated * [fixed] error message
1 parent 3f88f28 commit a95dfbb

File tree

5 files changed

+343
-0
lines changed

5 files changed

+343
-0
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ Enforce all the rules in this category, as well as all higher priority rules, wi
140140
| | [vue/no-template-key](./docs/rules/no-template-key.md) | disallow `key` attribute on `<template>` |
141141
| | [vue/no-textarea-mustache](./docs/rules/no-textarea-mustache.md) | disallow mustaches in `<textarea>` |
142142
| | [vue/no-unused-vars](./docs/rules/no-unused-vars.md) | disallow unused variable definitions of v-for directives or scope attributes |
143+
| | [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. |
143144
| | [vue/require-component-is](./docs/rules/require-component-is.md) | require `v-bind:is` of `<component>` elements |
144145
| | [vue/require-render-return](./docs/rules/require-render-return.md) | enforce render function to always return value |
145146
| | [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-for="todo in todos"
84+
v-if="todo.shown"
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-if="complete"
94+
v-for="todo in todos"
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
'prop-name-casing': require('./rules/prop-name-casing'),
3738
'require-component-is': require('./rules/require-component-is'),

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
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+
return !!getVForUsingIterationVar(vIf)
28+
}
29+
30+
function getVForUsingIterationVar (vIf) {
31+
const element = vIf.parent.parent
32+
for (var i = 0; i < vIf.value.references.length; i++) {
33+
const reference = vIf.value.references[i]
34+
35+
const targetVFor = element.variables.find(variable =>
36+
variable.id.name === reference.id.name &&
37+
variable.kind === 'v-for'
38+
)
39+
if (targetVFor) {
40+
return targetVFor.id.parent
41+
}
42+
}
43+
return undefined
44+
}
45+
46+
// ------------------------------------------------------------------------------
47+
// Rule Definition
48+
// ------------------------------------------------------------------------------
49+
50+
module.exports = {
51+
meta: {
52+
docs: {
53+
description: 'disallow use v-if on the same element as v-for',
54+
category: undefined,
55+
url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v4.3.0/docs/rules/no-use-v-if-with-v-for.md'
56+
},
57+
fixable: null,
58+
schema: [{
59+
type: 'object',
60+
properties: {
61+
allowUsingIterationVar: {
62+
type: 'boolean'
63+
}
64+
}
65+
}]
66+
},
67+
68+
create (context) {
69+
const options = context.options[0] || {}
70+
const allowUsingIterationVar = options.allowUsingIterationVar === true // default false
71+
return utils.defineTemplateBodyVisitor(context, {
72+
"VAttribute[directive=true][key.name='if']" (node) {
73+
const element = node.parent.parent
74+
75+
if (utils.hasDirective(element, 'for')) {
76+
if (isUsingIterationVar(node)) {
77+
if (!allowUsingIterationVar) {
78+
const vFor = getVForUsingIterationVar(node)
79+
context.report({
80+
node,
81+
loc: node.loc,
82+
message: "The '{{iteratorName}}' variable inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
83+
data: {
84+
iteratorName: vFor.right.name
85+
}
86+
})
87+
}
88+
} else {
89+
context.report({
90+
node,
91+
loc: node.loc,
92+
message: "This 'v-if' should be moved to the wrapper element."
93+
})
94+
}
95+
}
96+
}
97+
})
98+
}
99+
}

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

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
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: [{
83+
message: "This 'v-if' should be moved to the wrapper element.",
84+
line: 1
85+
}]
86+
},
87+
{
88+
filename: 'test.vue',
89+
code: '<template><div><div v-for="x in list" v-if="list.length&gt;0"></div></div></template>',
90+
errors: [{
91+
message: "This 'v-if' should be moved to the wrapper element.",
92+
line: 1
93+
}]
94+
},
95+
{
96+
filename: 'test.vue',
97+
code: '<template><div><div v-for="x in list" v-if="x.isActive"></div></div></template>',
98+
errors: [{
99+
message: "The 'list' variable inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
100+
line: 1
101+
}]
102+
},
103+
{
104+
filename: 'test.vue',
105+
code: `
106+
<template>
107+
<ul>
108+
<li
109+
v-for="user in users"
110+
v-if="user.isActive"
111+
:key="user.id"
112+
>
113+
{{ user.name }}
114+
<li>
115+
</ul>
116+
</template>
117+
`,
118+
errors: [{
119+
message: "The 'users' variable inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.",
120+
line: 6
121+
}]
122+
},
123+
{
124+
filename: 'test.vue',
125+
code: `
126+
<template>
127+
<ul>
128+
<li
129+
v-for="user in users"
130+
v-if="shouldShowUsers"
131+
:key="user.id"
132+
>
133+
{{ user.name }}
134+
<li>
135+
</ul>
136+
</template>
137+
`,
138+
errors: [{
139+
message: "This 'v-if' should be moved to the wrapper element.",
140+
line: 6
141+
}]
142+
}
143+
]
144+
})

0 commit comments

Comments
 (0)