From aa17e775b189782cc5d5d3f42f1edf92b2e4f587 Mon Sep 17 00:00:00 2001 From: Armano Date: Wed, 19 Jul 2017 17:57:37 +0200 Subject: [PATCH 1/3] Add `no-async-in-computed-properties` rule. --- docs/rules/no-async-in-computed-properties.md | 64 +++ lib/rules/no-async-in-computed-properties.js | 173 ++++++++ .../no-side-effects-in-computed-properties.js | 1 + .../rules/no-async-in-computed-properties.js | 369 ++++++++++++++++++ .../no-side-effects-in-computed-properties.js | 1 + 5 files changed, 608 insertions(+) create mode 100644 docs/rules/no-async-in-computed-properties.md create mode 100644 lib/rules/no-async-in-computed-properties.js create mode 100644 tests/lib/rules/no-async-in-computed-properties.js diff --git a/docs/rules/no-async-in-computed-properties.md b/docs/rules/no-async-in-computed-properties.md new file mode 100644 index 000000000..8c766c1c3 --- /dev/null +++ b/docs/rules/no-async-in-computed-properties.md @@ -0,0 +1,64 @@ +# Check if there are no asynchronous actions inside computed properties (no-async-in-computed-properties) + +Computed properties should be synchronous. Asynchronous actions inside them may not work as expected and can lead to an unexpected behaviour, that's why you should avoid them. +If you need async computed properties you might want to consider using additional plugin [vue-async-computed] + +## :book: Rule Details + +This rule is aimed at preventing asynchronous methods from being called in computed properties. + +:-1: Examples of **incorrect** code for this rule: + +```js +export default { + computed: { + pro () { + return Promise.all([new Promise((resolve, reject) => {})]) + }, + foo: async function () { + return await someFunc() + }, + bar () { + return fetch(url).then(response => {}) + }, + yiel: function* () { + yield 1 + yield* g1() + }, + tim () { + setTimeout(() => { }, 0) + }, + inter () { + setInterval(() => { }, 0) + }, + anim () { + requestAnimationFrame(() => {}) + } + } +} +``` + +:+1: Examples of **correct** code for this rule: + +```js +export default { + computed: { + foo () { + var bar = 0 + try { + bar = bar / this.a + } catch (e) { + return 0 + } finally { + return bar + } + } + } +} +``` + +## :wrench: Options + +Nothing. + +[vue-async-computed]: https://github.com/foxbenjaminfox/vue-async-computed diff --git a/lib/rules/no-async-in-computed-properties.js b/lib/rules/no-async-in-computed-properties.js new file mode 100644 index 000000000..023ab6df6 --- /dev/null +++ b/lib/rules/no-async-in-computed-properties.js @@ -0,0 +1,173 @@ +/** + * @fileoverview Check if there are no asynchronous actions inside computed properties. + * @author Armano + */ +'use strict' + +const utils = require('../utils') + +const PROMISE_FUNCTIONS = [ + 'then', + 'catch', + 'finally' +] + +const PROMISE_METHODS = [ + 'all', + 'race', + 'reject', + 'resolve' +] + +const TIMED_FUNCTIONS = [ + 'setTimeout', + 'setInterval', + 'setImmediate', + 'requestAnimationFrame' +] + +function isTimedFunction (node) { + return ( + node.type === 'CallExpression' && + node.callee.type === 'Identifier' && + TIMED_FUNCTIONS.indexOf(node.callee.name) !== -1 + ) || ( + node.type === 'CallExpression' && + node.callee.type === 'MemberExpression' && + node.callee.object.type === 'Identifier' && + node.callee.object.name === 'window' && ( + TIMED_FUNCTIONS.indexOf(node.callee.property.name) !== -1 + ) + ) && node.arguments.length +} + +function isPromise (node) { + if (node.type === 'CallExpression' && node.callee.type === 'MemberExpression') { + return ( // hello.PROMISE_FUNCTION() + node.callee.property.type === 'Identifier' && + PROMISE_FUNCTIONS.indexOf(node.callee.property.name) !== -1 + ) || ( // Promise.PROMISE_METHOD() + node.callee.object.type === 'Identifier' && + node.callee.object.name === 'Promise' && + PROMISE_METHODS.indexOf(node.callee.property.name) !== -1 + ) + } + return false +} + +function create (context) { + const forbiddenNodes = [] + + const expressionTypes = { + promise: 'asynchronous action', + await: 'await operator', + yield: 'yield keyword', + generator: 'generator function expression', + async: 'async function declaration', + new: 'Promise object', + timed: 'timed function' + } + + function onFunctionEnter (node) { + if (node.async) { + forbiddenNodes.push({ + node: node, + type: 'async' + }) + } + if (node.generator) { + forbiddenNodes.push({ + node: node, + type: 'generator' + }) + } + } + + return Object.assign({}, + { + FunctionDeclaration: onFunctionEnter, + + FunctionExpression: onFunctionEnter, + + ArrowFunctionExpression: onFunctionEnter, + + NewExpression (node) { + if (node.callee.name === 'Promise') { + forbiddenNodes.push({ + node: node, + type: 'new' + }) + } + }, + + CallExpression (node) { + if (isPromise(node)) { + forbiddenNodes.push({ + node: node, + type: 'promise' + }) + } + if (isTimedFunction(node)) { + forbiddenNodes.push({ + node: node, + type: 'timed' + }) + } + }, + + YieldExpression (node) { + // await nodes are YieldExpression's with babel-eslint < 7.0.0 + forbiddenNodes.push({ + node: node, + type: 'yield' + }) + }, + + AwaitExpression (node) { + forbiddenNodes.push({ + node: node, + type: 'await' + }) + } + }, + utils.executeOnVueComponent(context, (obj) => { + const computedProperties = utils.getComputedProperties(obj) + + computedProperties.forEach(cp => { + forbiddenNodes.forEach(el => { + if ( + cp.value && + el.node.loc.start.line >= cp.value.loc.start.line && + el.node.loc.end.line <= cp.value.loc.end.line + ) { + context.report({ + node: el.node, + message: 'Unexpected {{expressionName}} in "{{propertyName}}" computed property.', + data: { + expressionName: expressionTypes[el.type], + propertyName: cp.key + } + }) + } + }) + }) + }) + ) +} + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + create, + meta: { + docs: { + description: 'Check if there are no asynchronous actions inside computed properties.', + category: 'Best Practices', + recommended: false + }, + fixable: null, + schema: [] + } +} diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index 968f7a814..beb928ad8 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -39,6 +39,7 @@ function create (context) { computedProperties.forEach(cp => { forbiddenNodes.forEach(node => { if ( + cp.value && node.loc.start.line >= cp.value.loc.start.line && node.loc.end.line <= cp.value.loc.end.line ) { diff --git a/tests/lib/rules/no-async-in-computed-properties.js b/tests/lib/rules/no-async-in-computed-properties.js new file mode 100644 index 000000000..6d7ef32f9 --- /dev/null +++ b/tests/lib/rules/no-async-in-computed-properties.js @@ -0,0 +1,369 @@ +/** + * @fileoverview Check if there are no side effects inside computed properties + * @author 2017 Armano + */ +'use strict' + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-async-in-computed-properties') +const RuleTester = require('eslint').RuleTester + +const parserOptions = { + ecmaVersion: 6, + sourceType: 'module', + ecmaFeatures: { experimentalObjectRestSpread: true } +} + +const parserOptions8 = { + ecmaVersion: 8, + sourceType: 'module', + ecmaFeatures: { experimentalObjectRestSpread: true } +} + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester() + +ruleTester.run('no-async-in-computed-properties', rule, { + + valid: [ + { + filename: 'test.vue', + code: ` + export default { + computed: { + ...mapGetters({ + test: 'getTest' + }), + foo: function () { + var bar = 0 + try { + bar = bar / 0 + } catch (e) { + return e + } finally { + return bar + } + }, + foo2: { + set () { + new Promise((resolve, reject) => {}) + } + } + } + } + `, + parserOptions + } + ], + + invalid: [ + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: async function () { + return await someFunc() + } + } + } + `, + parserOptions: parserOptions8, + errors: [{ + message: 'Unexpected async function declaration in "foo" computed property.', + line: 4 + }, { + message: 'Unexpected await operator in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: async function () { + return new Promise((resolve, reject) => {}) + } + } + } + `, + parserOptions: parserOptions8, + errors: [{ + message: 'Unexpected async function declaration in "foo" computed property.', + line: 4 + }, { + message: 'Unexpected Promise object in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function () { + return bar.then(response => {}) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function () { + return bar.catch(e => {}) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function () { + return Promise.all([]) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function () { + return bar.finally(res => {}) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function () { + return Promise.race([]) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function () { + return Promise.reject([]) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function () { + return Promise.resolve([]) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo () { + return Promise.resolve([]) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 5 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: { + get () { + return Promise.resolve([]) + } + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 6 + }] + }, + { + filename: 'test.vue', + code: ` + new Vue({ + computed: { + foo: { + get () { + return Promise.resolve([]) + } + } + } + }) + `, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 6 + }] + }, + { + filename: 'test.vue', + code: ` + new Vue({ + computed: { + foo: { + get () { + return test.blabla.then([]) + } + } + } + }) + `, + parserOptions: { ecmaVersion: 6 }, + errors: [{ + message: 'Unexpected asynchronous action in "foo" computed property.', + line: 6 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function* () { + yield 1 + yield* g1() + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected generator function expression in "foo" computed property.', + line: 4 + }, { + message: 'Unexpected yield keyword in "foo" computed property.', + line: 5 + }, { + message: 'Unexpected yield keyword in "foo" computed property.', + line: 6 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + computed: { + foo: function () { + setTimeout(() => { }, 0) + window.setTimeout(() => { }, 0) + setInterval(() => { }, 0) + window.setInterval(() => { }, 0) + setImmediate(() => { }) + window.setImmediate(() => { }) + requestAnimationFrame(() => {}) + window.requestAnimationFrame(() => {}) + } + } + } + `, + parserOptions, + errors: [{ + message: 'Unexpected timed function in "foo" computed property.', + line: 5 + }, { + message: 'Unexpected timed function in "foo" computed property.', + line: 6 + }, { + message: 'Unexpected timed function in "foo" computed property.', + line: 7 + }, { + message: 'Unexpected timed function in "foo" computed property.', + line: 8 + }, { + message: 'Unexpected timed function in "foo" computed property.', + line: 9 + }, { + message: 'Unexpected timed function in "foo" computed property.', + line: 10 + }, { + message: 'Unexpected timed function in "foo" computed property.', + line: 11 + }, { + message: 'Unexpected timed function in "foo" computed property.', + line: 12 + }] + } + ] +}) diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js index 5a58de081..c6f95f15d 100644 --- a/tests/lib/rules/no-side-effects-in-computed-properties.js +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -27,6 +27,7 @@ ruleTester.run('no-side-effects-in-computed-properties', rule, { { code: `Vue.component('test', { computed: { + ...test0({}), test1() { return this.firstName + ' ' + this.lastName }, From 76695d0dbae7e620646a68e3b4503c1e5e17fa49 Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 21 Jul 2017 15:49:00 +0200 Subject: [PATCH 2/3] Remove yield/generator from this rule. --- lib/rules/no-async-in-computed-properties.js | 16 ------------- .../rules/no-async-in-computed-properties.js | 24 ------------------- 2 files changed, 40 deletions(-) diff --git a/lib/rules/no-async-in-computed-properties.js b/lib/rules/no-async-in-computed-properties.js index 023ab6df6..46e5e88e7 100644 --- a/lib/rules/no-async-in-computed-properties.js +++ b/lib/rules/no-async-in-computed-properties.js @@ -61,8 +61,6 @@ function create (context) { const expressionTypes = { promise: 'asynchronous action', await: 'await operator', - yield: 'yield keyword', - generator: 'generator function expression', async: 'async function declaration', new: 'Promise object', timed: 'timed function' @@ -75,12 +73,6 @@ function create (context) { type: 'async' }) } - if (node.generator) { - forbiddenNodes.push({ - node: node, - type: 'generator' - }) - } } return Object.assign({}, @@ -115,14 +107,6 @@ function create (context) { } }, - YieldExpression (node) { - // await nodes are YieldExpression's with babel-eslint < 7.0.0 - forbiddenNodes.push({ - node: node, - type: 'yield' - }) - }, - AwaitExpression (node) { forbiddenNodes.push({ node: node, diff --git a/tests/lib/rules/no-async-in-computed-properties.js b/tests/lib/rules/no-async-in-computed-properties.js index 6d7ef32f9..34c57e75f 100644 --- a/tests/lib/rules/no-async-in-computed-properties.js +++ b/tests/lib/rules/no-async-in-computed-properties.js @@ -296,30 +296,6 @@ ruleTester.run('no-async-in-computed-properties', rule, { line: 6 }] }, - { - filename: 'test.vue', - code: ` - export default { - computed: { - foo: function* () { - yield 1 - yield* g1() - } - } - } - `, - parserOptions, - errors: [{ - message: 'Unexpected generator function expression in "foo" computed property.', - line: 4 - }, { - message: 'Unexpected yield keyword in "foo" computed property.', - line: 5 - }, { - message: 'Unexpected yield keyword in "foo" computed property.', - line: 6 - }] - }, { filename: 'test.vue', code: ` From 67c10865bc8b76fc77047da24075fa6353a43c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Sat, 22 Jul 2017 13:27:51 +0200 Subject: [PATCH 3/3] Remove yield from example code --- docs/rules/no-async-in-computed-properties.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/rules/no-async-in-computed-properties.md b/docs/rules/no-async-in-computed-properties.md index 8c766c1c3..83c48b59e 100644 --- a/docs/rules/no-async-in-computed-properties.md +++ b/docs/rules/no-async-in-computed-properties.md @@ -21,10 +21,6 @@ export default { bar () { return fetch(url).then(response => {}) }, - yiel: function* () { - yield 1 - yield* g1() - }, tim () { setTimeout(() => { }, 0) },