Skip to content

[New] Add rule vue/no-use-v-if-with-v-for #2

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
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -138,6 +138,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:

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

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

```html
<TodoItem
v-for="todo in todos"
v-if="todo.shown"
: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'),
'require-component-is': require('./rules/require-component-is'),
'require-default-prop': require('./rules/require-default-prop'),
Expand Down
85 changes: 85 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,85 @@
/**
* @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) {
const element = vIf.parent.parent
return vIf.value.references.some(reference =>
element.variables.some(variable =>
variable.id.name === reference.id.name &&
variable.kind === 'v-for'
)
)
}

// ------------------------------------------------------------------------------
// 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.2.2/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) {
context.report({
node,
loc: node.loc,
message: "The 'v-for' list variable should be replace with a new computed property that returns your filtered list by this 'v-if' condition."
})
}
} else {
context.report({
node,
loc: node.loc,
message: "This 'v-if' should be moved to the wrapper element."
})
}
}
}
})
}
}
129 changes: 129 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,129 @@
/**
* @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: ["This 'v-if' should be moved to the wrapper element."]
},
{
filename: 'test.vue',
code: '<template><div><div v-for="x in list" v-if="list.length&gt;0"></div></div></template>',
errors: ["This 'v-if' should be moved to the wrapper element."]
},
{
filename: 'test.vue',
code: '<template><div><div v-for="x in list" v-if="x.isActive"></div></div></template>',
errors: ["The 'v-for' list variable should be replace with a new computed property that returns your filtered list by this 'v-if' condition."]
},
{
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: ["The 'v-for' list variable should be replace with a new computed property that returns your filtered list by this 'v-if' condition."]
},
{
filename: 'test.vue',
code: `
<template>
<ul>
<li
v-for="user in users"
v-if="shouldShowUsers"
:key="user.id"
>
{{ user.name }}
<li>
</ul>
</template>
`,
errors: ["This 'v-if' should be moved to the wrapper element."]
}
]
})