Skip to content

⭐️New: Add vue/no-use-v-if-with-v-for rule #406

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
merged 23 commits into from
Jul 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
98dbf8a
Add Vue.extend support, add missing info about Vue.mixin check in readme
michalsnik Jan 2, 2018
7c4a1d2
Docs: fixes wording in docs (#372)
samturrell Feb 1, 2018
cd22a28
Fix: fix script-indent to prevent removing <script> tag (fixes #367) …
mysticatea Feb 16, 2018
ee5cc0a
[Update] Make `vue/max-attributes-per-line` fixable (#380)
ota-meshi Feb 16, 2018
6861c81
Update: make `vue/order-in-components` fixable (#381)
ota-meshi Feb 17, 2018
5890808
Fix: no-async-in-computed-properties crash (fixes #390)
mysticatea Feb 16, 2018
e79dd8b
Fix: valid-v-on false positive (fixes #351)
mysticatea Feb 16, 2018
733172c
Fix: indent rules false positive (fixes #375)
mysticatea Feb 16, 2018
e887a8d
[New] Add `prop-name-casing`
Dec 10, 2017
22a3a75
fix message and category
Jan 4, 2018
a846dd1
fix category
Jan 4, 2018
b1a7cca
fix test message
Jan 4, 2018
17d8c2b
Set category to undefined
michalsnik Jan 6, 2018
789a2fc
Add more tests and fix edge case scenario
michalsnik Jan 28, 2018
e804ca2
attribute order linting
Jan 5, 2018
abbde6f
updating docs, tests and category
Feb 7, 2018
60a36c2
[New] Add rule `vue/no-use-v-if-with-v-for` (#2)
ota-meshi Feb 23, 2018
9105525
[fix] `meta.docs.description` should not end with `.` consistent-doc…
ota-meshi Feb 23, 2018
00c319c
Merge branch 'master' into add-no-use-v-if-with-v-for
ota-meshi Feb 24, 2018
c4bcc5c
[fix] lint error caused by merging the master for conflict resolution
ota-meshi Feb 24, 2018
342c049
[fix] That `vue/attributes-order` got duplicated when merging README
ota-meshi Feb 24, 2018
2767798
[fixed] document `correct` and `incorrect` the contrary stated
ota-meshi Mar 23, 2018
6709702
[fixed] error message
ota-meshi Mar 23, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ Enforce all the rules in this category, as well as all higher priority rules, wi
| | [vue/no-template-key](./docs/rules/no-template-key.md) | disallow `key` attribute on `<template>` |
| | [vue/no-textarea-mustache](./docs/rules/no-textarea-mustache.md) | disallow mustaches in `<textarea>` |
| | [vue/no-unused-vars](./docs/rules/no-unused-vars.md) | disallow unused variable definitions of v-for directives or scope attributes |
| | [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. |
| | [vue/require-component-is](./docs/rules/require-component-is.md) | require `v-bind:is` of `<component>` elements |
| | [vue/require-render-return](./docs/rules/require-render-return.md) | enforce render function to always return value |
| | [vue/require-v-for-key](./docs/rules/require-v-for-key.md) | require `v-bind:key` with `v-for` directives |
Expand Down
98 changes: 98 additions & 0 deletions docs/rules/no-use-v-if-with-v-for.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# disallow use v-if on the same element as v-for. (vue/no-use-v-if-with-v-for)

> Never use `v-if` on the same element as `v-for`.
>
> There are two common cases where this can be tempting:
>
> * 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`).
>
> * 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`).
>
> https://vuejs.org/v2/style-guide/#Avoid-v-if-with-v-for-essential


## :book: Rule Details

:-1: Examples of **incorrect** code for this rule:

```html
<TodoItem
v-if="complete"
v-for="todo in todos"
:todo="todo"
/>
```

In this case, the `v-if` should be written on the wrapper element.


```html
<TodoItem
v-for="todo in todos"
v-if="todo.shown"
:todo="todo"
/>
```

In this case, the `v-for` list variable should be replace with a computed property that returns your filtered list.


:+1: Examples of **correct** code for this rule:


```html
<ul v-if="complete">
<TodoItem
v-for="todo in todos"
:todo="todo"
/>
</ul>
```



```html
<TodoItem
v-for="todo in shownTodos"
:todo="todo"
/>
```

```js
computed: {
shownTodos() {
return this.todos.filter((todo) => todo.shown)
}
}
```

## :wrench: Options

`allowUsingIterationVar` - Enables The `v-if` directive use the reference which is to the variables which are defined by the `v-for` directives.

```js
'vue/no-use-v-if-with-v-for': ['error', {
allowUsingIterationVar: true // default: false
}]
```

:+1: Examples of additional **correct** code for this rule with sample `"allowUsingIterationVar": true` options:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is supposed to be incorrect right?


```html
<TodoItem
v-for="todo in todos"
v-if="todo.shown"
:todo="todo"
/>
```

:-1: Examples of additional **incorrect** code for this rule with sample `"allowUsingIterationVar": true` options:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this correct


```html
<TodoItem
v-if="complete"
v-for="todo in todos"
:todo="todo"
/>
```

1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ module.exports = {
'no-template-key': require('./rules/no-template-key'),
'no-textarea-mustache': require('./rules/no-textarea-mustache'),
'no-unused-vars': require('./rules/no-unused-vars'),
'no-use-v-if-with-v-for': require('./rules/no-use-v-if-with-v-for'),
'order-in-components': require('./rules/order-in-components'),
'prop-name-casing': require('./rules/prop-name-casing'),
'require-component-is': require('./rules/require-component-is'),
Expand Down
99 changes: 99 additions & 0 deletions lib/rules/no-use-v-if-with-v-for.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/**
* @author Yosuke Ota
*
* issue https://github.com/vuejs/eslint-plugin-vue/issues/403
* Style guide: https://vuejs.org/v2/style-guide/#Avoid-v-if-with-v-for-essential
*
* I implemented it with reference to `no-confusing-v-for-v-if`
*/
'use strict'

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

const utils = require('../utils')

// ------------------------------------------------------------------------------
// Helpers
// ------------------------------------------------------------------------------

/**
* Check whether the given `v-if` node is using the variable which is defined by the `v-for` directive.
* @param {ASTNode} vIf The `v-if` attribute node to check.
* @returns {boolean} `true` if the `v-if` is using the variable which is defined by the `v-for` directive.
*/
function isUsingIterationVar (vIf) {
return !!getVForUsingIterationVar(vIf)
}

function getVForUsingIterationVar (vIf) {
const element = vIf.parent.parent
for (var i = 0; i < vIf.value.references.length; i++) {
const reference = vIf.value.references[i]

const targetVFor = element.variables.find(variable =>
variable.id.name === reference.id.name &&
variable.kind === 'v-for'
)
if (targetVFor) {
return targetVFor.id.parent
}
}
return undefined
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'disallow use v-if on the same element as v-for',
category: undefined,
url: 'https://github.com/vuejs/eslint-plugin-vue/blob/v4.3.0/docs/rules/no-use-v-if-with-v-for.md'
},
fixable: null,
schema: [{
type: 'object',
properties: {
allowUsingIterationVar: {
type: 'boolean'
}
}
}]
},

create (context) {
const options = context.options[0] || {}
const allowUsingIterationVar = options.allowUsingIterationVar === true // default false
return utils.defineTemplateBodyVisitor(context, {
"VAttribute[directive=true][key.name='if']" (node) {
const element = node.parent.parent

if (utils.hasDirective(element, 'for')) {
if (isUsingIterationVar(node)) {
if (!allowUsingIterationVar) {
const vFor = getVForUsingIterationVar(node)
context.report({
node,
loc: node.loc,
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'.",
data: {
iteratorName: vFor.right.name
}
})
}
} else {
context.report({
node,
loc: node.loc,
message: "This 'v-if' should be moved to the wrapper element."
})
}
}
}
})
}
}
144 changes: 144 additions & 0 deletions tests/lib/rules/no-use-v-if-with-v-for.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/**
* @author Yosuke Ota
*/
'use strict'

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

const RuleTester = require('eslint').RuleTester
const rule = require('../../../lib/rules/no-use-v-if-with-v-for')

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------

const tester = new RuleTester({
parser: 'vue-eslint-parser',
parserOptions: { ecmaVersion: 2015 }
})

tester.run('no-use-v-if-with-v-for', rule, {
valid: [
{
filename: 'test.vue',
code: ''
},
{
filename: 'test.vue',
code: '<template><div><div v-for="x in list" v-if="x"></div></div></template>',
options: [{ allowUsingIterationVar: true }]
},
{
filename: 'test.vue',
code: '<template><div><div v-for="x in list" v-if="x.foo"></div></div></template>',
options: [{ allowUsingIterationVar: true }]
},
{
filename: 'test.vue',
code: '<template><div><div v-for="(x,i) in list" v-if="i%2==0"></div></div></template>',
options: [{ allowUsingIterationVar: true }]
},
{
filename: 'test.vue',
code: '<template><div v-if="shown"><div v-for="(x,i) in list"></div></div></template>'
},
{
filename: 'test.vue',
code: `
<template>
<ul>
<li
v-for="user in activeUsers"
:key="user.id"
>
{{ user.name }}
<li>
</ul>
</template>
`
},
{
filename: 'test.vue',
code: `
<template>
<ul v-if="shouldShowUsers">
<li
v-for="user in users"
:key="user.id"
>
{{ user.name }}
<li>
</ul>
</template>
`
}
],
invalid: [
{
filename: 'test.vue',
code: '<template><div><div v-for="x in list" v-if="shown"></div></div></template>',
errors: [{
message: "This 'v-if' should be moved to the wrapper element.",
line: 1
}]
},
{
filename: 'test.vue',
code: '<template><div><div v-for="x in list" v-if="list.length&gt;0"></div></div></template>',
errors: [{
message: "This 'v-if' should be moved to the wrapper element.",
line: 1
}]
},
{
filename: 'test.vue',
code: '<template><div><div v-for="x in list" v-if="x.isActive"></div></div></template>',
errors: [{
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'.",
line: 1
}]
},
{
filename: 'test.vue',
code: `
<template>
<ul>
<li
v-for="user in users"
v-if="user.isActive"
:key="user.id"
>
{{ user.name }}
<li>
</ul>
</template>
`,
errors: [{
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'.",
line: 6
}]
},
{
filename: 'test.vue',
code: `
<template>
<ul>
<li
v-for="user in users"
v-if="shouldShowUsers"
:key="user.id"
>
{{ user.name }}
<li>
</ul>
</template>
`,
errors: [{
message: "This 'v-if' should be moved to the wrapper element.",
line: 6
}]
}
]
})