From cce26efb3c22630724750e691072f954f0b49a6d Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Sat, 24 Feb 2018 02:48:04 +0900 Subject: [PATCH 1/9] [New] Add rule `vue/no-use-v-if-with-v-for` --- lib/rules/no-use-v-if-with-v-for.js | 85 +++++++++++++++++++ tests/lib/rules/no-use-v-if-with-v-for.js | 99 +++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 lib/rules/no-use-v-if-with-v-for.js create mode 100644 tests/lib/rules/no-use-v-if-with-v-for.js diff --git a/lib/rules/no-use-v-if-with-v-for.js b/lib/rules/no-use-v-if-with-v-for.js new file mode 100644 index 000000000..bcd34d364 --- /dev/null +++ b/lib/rules/no-use-v-if-with-v-for.js @@ -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 by this 'v-if' condition." + }) + } + } else { + context.report({ + node, + loc: node.loc, + message: "This 'v-if' should be moved to the wrapper element." + }) + } + } + } + }) + } +} diff --git a/tests/lib/rules/no-use-v-if-with-v-for.js b/tests/lib/rules/no-use-v-if-with-v-for.js new file mode 100644 index 000000000..19a6f7b30 --- /dev/null +++ b/tests/lib/rules/no-use-v-if-with-v-for.js @@ -0,0 +1,99 @@ +/** + * @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: '', + options: [{ allowUsingIterationVar: true }] + }, + { + filename: 'test.vue', + code: '', + options: [{ allowUsingIterationVar: true }] + }, + { + filename: 'test.vue', + code: '', + options: [{ allowUsingIterationVar: true }] + }, + { + filename: 'test.vue', + code: '' + } + ], + invalid: [ + { + filename: 'test.vue', + code: '', + errors: ["This 'v-if' should be moved to the wrapper element."] + }, + { + filename: 'test.vue', + code: '', + errors: ["This 'v-if' should be moved to the wrapper element."] + }, + { + filename: 'test.vue', + code: '', + errors: ["The 'v-for' list variable should be replace with a new computed property that returns your filtered by this 'v-if' condition."] + }, + { + filename: 'test.vue', + code: ` + + `, + errors: ["The 'v-for' list variable should be replace with a new computed property that returns your filtered by this 'v-if' condition."] + }, + { + filename: 'test.vue', + code: ` + + `, + errors: ["This 'v-if' should be moved to the wrapper element."] + } + ] +}) From 51485ed3495a060c93b94e3d87a64f01728d5dbe Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Sat, 24 Feb 2018 02:51:06 +0900 Subject: [PATCH 2/9] [add] guide example test --- tests/lib/rules/no-use-v-if-with-v-for.js | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/lib/rules/no-use-v-if-with-v-for.js b/tests/lib/rules/no-use-v-if-with-v-for.js index 19a6f7b30..6133c04bf 100644 --- a/tests/lib/rules/no-use-v-if-with-v-for.js +++ b/tests/lib/rules/no-use-v-if-with-v-for.js @@ -43,6 +43,36 @@ tester.run('no-use-v-if-with-v-for', rule, { { filename: 'test.vue', code: '' + }, + { + filename: 'test.vue', + code: ` + + ` + }, + { + filename: 'test.vue', + code: ` + + ` } ], invalid: [ From 0ce3e0622764e4cfadfa853e36cb40c9945b18ad Mon Sep 17 00:00:00 2001 From: yosuke ota Date: Sat, 24 Feb 2018 03:37:26 +0900 Subject: [PATCH 3/9] [update] documents --- README.md | 1 + docs/rules/no-use-v-if-with-v-for.md | 87 +++++++++++++++++++++++ lib/index.js | 1 + lib/rules/no-use-v-if-with-v-for.js | 2 +- tests/lib/rules/no-use-v-if-with-v-for.js | 4 +- 5 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 docs/rules/no-use-v-if-with-v-for.md diff --git a/README.md b/README.md index 0ff269645..5436f1fc8 100644 --- a/README.md +++ b/README.md @@ -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 `