Skip to content
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

Add composition api's computed function support to vue/no-side-effects-in-computed-properties close #1393 #1407

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 49 additions & 4 deletions docs/rules/no-side-effects-in-computed-properties.md
Expand Up @@ -2,20 +2,20 @@
pageClass: rule-details
sidebarDepth: 0
title: vue/no-side-effects-in-computed-properties
description: disallow side effects in computed properties
description: disallow side effects in computed properties and functions
since: v3.6.0
---
# vue/no-side-effects-in-computed-properties

> disallow side effects in computed properties
> disallow side effects in computed properties and functions

- :gear: This rule is included in all of `"plugin:vue/vue3-essential"`, `"plugin:vue/essential"`, `"plugin:vue/vue3-strongly-recommended"`, `"plugin:vue/strongly-recommended"`, `"plugin:vue/vue3-recommended"` and `"plugin:vue/recommended"`.

## :book: Rule Details

This rule is aimed at preventing the code which makes side effects in computed properties.
This rule is aimed at preventing the code which makes side effects in computed properties and functions.

It is considered a very bad practice to introduce side effects inside computed properties. It makes the code not predictable and hard to understand.
It is considered a very bad practice to introduce side effects inside computed properties and functions. It makes the code not predictable and hard to understand.

<eslint-code-block :rules="{'vue/no-side-effects-in-computed-properties': ['error']}">

Expand Down Expand Up @@ -58,6 +58,51 @@ export default {

</eslint-code-block>

<eslint-code-block :rules="{'vue/no-side-effects-in-computed-properties': ['error']}">

```vue
<script>
import {computed} from 'vue'
/* ✓ GOOD */
export default {
setup() {
const foo = useFoo()

const fullName = computed(() => `${foo.firstName} ${foo.lastName}`)
const reversedArray = computed(() => {
return foo.array.slice(0).reverse() // .slice makes a copy of the array, instead of mutating the orginal
})
}
}
</script>
```

</eslint-code-block>

<eslint-code-block :rules="{'vue/no-side-effects-in-computed-properties': ['error']}">

```vue
<script>
import {computed} from 'vue'
/* ✗ BAD */
export default {
setup() {
const foo = useFoo()

const fullName = computed(() => {
foo.firstName = 'lorem' // <- side effect
return `${foo.firstName} ${foo.lastName}`
})
const reversedArray = computed(() => {
return foo.array.reverse() // <- side effect - orginal array is being mutated
})
}
}
</script>
```

</eslint-code-block>

## :wrench: Options

Nothing.
Expand Down
185 changes: 138 additions & 47 deletions lib/rules/no-side-effects-in-computed-properties.js
Expand Up @@ -3,7 +3,7 @@
* @author Michał Sajnóg
*/
'use strict'

const { ReferenceTracker, findVariable } = require('eslint-utils')
const utils = require('../utils')

/**
Expand Down Expand Up @@ -31,6 +31,10 @@ module.exports = {
create(context) {
/** @type {Map<ObjectExpression, ComponentComputedProperty[]>} */
const computedPropertiesMap = new Map()
/** @type {Array<FunctionExpression | ArrowFunctionExpression>} */
const computedCallNodes = []
/** @type {Array<FunctionExpression | ArrowFunctionExpression | FunctionDeclaration>} */
const setupFunctions = []

/**
* @typedef {object} ScopeStack
Expand All @@ -54,56 +58,143 @@ module.exports = {
scopeStack = scopeStack && scopeStack.upper
}

return utils.defineVueVisitor(context, {
onVueObjectEnter(node) {
computedPropertiesMap.set(node, utils.getComputedProperties(node))
},
':function': onFunctionEnter,
':function:exit': onFunctionExit,

/**
* @param {(Identifier | ThisExpression) & {parent: MemberExpression}} node
* @param {VueObjectData} data
*/
'MemberExpression > :matches(Identifier, ThisExpression)'(
node,
{ node: vueNode }
) {
if (!scopeStack) {
return
}
const targetBody = scopeStack.body
const computedProperty = /** @type {ComponentComputedProperty[]} */ (computedPropertiesMap.get(
vueNode
)).find((cp) => {
return (
cp.value &&
node.loc.start.line >= cp.value.loc.start.line &&
node.loc.end.line <= cp.value.loc.end.line &&
targetBody === cp.value
)
})
if (!computedProperty) {
return
}
return Object.assign(
{
Program() {
const tracker = new ReferenceTracker(context.getScope())
const traceMap = utils.createCompositionApiTraceMap({
[ReferenceTracker.ESM]: true,
computed: {
[ReferenceTracker.CALL]: true
}
})

if (!utils.isThis(node, context)) {
return
}
const mem = node.parent
if (mem.object !== node) {
return
for (const { node } of tracker.iterateEsmReferences(traceMap)) {
if (node.type !== 'CallExpression') {
continue
}

const getterBody = utils.getGetterBodyFromComputedFunction(node)
if (getterBody) {
computedCallNodes.push(getterBody)
}
}
}
},
utils.defineVueVisitor(context, {
onVueObjectEnter(node) {
computedPropertiesMap.set(node, utils.getComputedProperties(node))
},
':function': onFunctionEnter,
':function:exit': onFunctionExit,
onSetupFunctionEnter(node) {
setupFunctions.push(node)
},

const invalid = utils.findMutating(mem)
if (invalid) {
context.report({
node: invalid.node,
message: 'Unexpected side effect in "{{key}}" computed property.',
data: { key: computedProperty.key || 'Unknown' }
/**
* @param {(Identifier | ThisExpression) & {parent: MemberExpression}} node
* @param {VueObjectData} data
*/
'MemberExpression > :matches(Identifier, ThisExpression)'(
node,
{ node: vueNode }
) {
if (!scopeStack) {
return
}
const targetBody = scopeStack.body

const computedProperty = /** @type {ComponentComputedProperty[]} */ (computedPropertiesMap.get(
vueNode
)).find((cp) => {
return (
cp.value &&
node.loc.start.line >= cp.value.loc.start.line &&
node.loc.end.line <= cp.value.loc.end.line &&
targetBody === cp.value
)
})
if (computedProperty) {
if (!utils.isThis(node, context)) {
return
}
const mem = node.parent
if (mem.object !== node) {
return
}

const invalid = utils.findMutating(mem)
if (invalid) {
context.report({
node: invalid.node,
message:
'Unexpected side effect in "{{key}}" computed property.',
data: { key: computedProperty.key || 'Unknown' }
})
}
return
}

// ignore `this` for computed functions
if (node.type === 'ThisExpression') {
ota-meshi marked this conversation as resolved.
Show resolved Hide resolved
return
}

const computedFunction = computedCallNodes.find(
(c) =>
node.loc.start.line >= c.loc.start.line &&
node.loc.end.line <= c.loc.end.line &&
targetBody === c.body
)
if (!computedFunction) {
return
}

const mem = node.parent
if (mem.object !== node) {
return
}

const variable = findVariable(context.getScope(), node)
if (!variable || variable.defs.length !== 1) {
return
}

const def = variable.defs[0]
if (
def.type === 'ImplicitGlobalVariable' ||
def.type === 'TDZ' ||
def.type === 'ImportBinding'
) {
return
}

const isDeclaredInsideSetup = setupFunctions.some(
(setupFn) =>
def.node.loc.start.line >= setupFn.loc.start.line &&
def.node.loc.end.line <= setupFn.loc.end.line
)
if (!isDeclaredInsideSetup) {
return
}

if (
def.node.loc.start.line >= computedFunction.loc.start.line &&
def.node.loc.end.line <= computedFunction.loc.end.line
) {
// mutating local variables are accepted
return
}

const invalid = utils.findMutating(node)
if (invalid) {
context.report({
node: invalid.node,
message: 'Unexpected side effect in computed function.'
})
}
}
}
})
})
)
}
}