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

New Add vue/no-computed-in-data Rule #1075

Closed
Closed
Show file tree
Hide file tree
Changes from 12 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
1 change: 1 addition & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ Enforce all the rules in this category, as well as all higher priority rules, wi
| [vue/max-attributes-per-line](./max-attributes-per-line.md) | enforce the maximum number of attributes per line | :wrench: |
| [vue/multiline-html-element-content-newline](./multiline-html-element-content-newline.md) | require a line break before and after the contents of a multiline element | :wrench: |
| [vue/mustache-interpolation-spacing](./mustache-interpolation-spacing.md) | enforce unified spacing in mustache interpolations | :wrench: |
| [vue/no-computed-in-data](./no-computed-in-data.md) | require computed properties are not used in the data() | |
| [vue/no-multi-spaces](./no-multi-spaces.md) | disallow multiple spaces | :wrench: |
| [vue/no-spaces-around-equal-signs-in-attribute](./no-spaces-around-equal-signs-in-attribute.md) | disallow spaces around equal signs in attribute | :wrench: |
| [vue/no-template-shadow](./no-template-shadow.md) | disallow variable declarations from shadowing variables declared in the outer scope | |
Expand Down
81 changes: 81 additions & 0 deletions docs/rules/no-computed-in-data.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
---
pageClass: rule-details
sidebarDepth: 0
title: vue/no-computed-in-data
description: require computed properties are not used in the data()
---
# vue/no-computed-in-data
> require computed properties are not used in the data()

- :gear: This rule is included in `"plugin:vue/strongly-recommended"` and `"plugin:vue/recommended"`.

> require computed properties are not used in the data()
IWANABETHATGUY marked this conversation as resolved.
Show resolved Hide resolved

- :gear: This rule is included in `"plugin:vue/strongly-recommended"` and `"plugin:vue/recommended"`.

Please describe the origin of the rule here.

## :book: Rule Details

This rule report computed properties are used in data property

Examples of **incorrect** code for this rule:
IWANABETHATGUY marked this conversation as resolved.
Show resolved Hide resolved

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

```vue
<script>
export default {
data() {
return {
value: 'hello ' + this.world,
a: this.world,
b: this.world,
};
},
computed: {
world() {
return 'world';
},
},
};
</script>
```

</eslint-code-block>

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

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

```vue
<script>
export default {
data() {
return {
value: 'hello ' + 'world',
};
},
computed: {
world() {
return 'world';
},
},
};
</script>
```

</eslint-code-block>

## :wrench: Options

nothing

## Further Reading

[Computed Properties](https://vuejs.org/v2/guide/computed.html#Computed-Caching-vs-Methods)

## :mag: Implementation

- [Rule source](https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/no-computed-in-data.js)
- [Test source](https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/no-computed-in-data.js)
1 change: 1 addition & 0 deletions lib/configs/strongly-recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module.exports = {
'vue/max-attributes-per-line': 'warn',
'vue/multiline-html-element-content-newline': 'warn',
'vue/mustache-interpolation-spacing': 'warn',
'vue/no-computed-in-data': 'warn',
'vue/no-multi-spaces': 'warn',
'vue/no-spaces-around-equal-signs-in-attribute': 'warn',
'vue/no-template-shadow': 'warn',
Expand Down
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ module.exports = {
'name-property-casing': require('./rules/name-property-casing'),
'no-async-in-computed-properties': require('./rules/no-async-in-computed-properties'),
'no-boolean-default': require('./rules/no-boolean-default'),
'no-computed-in-data': require('./rules/no-computed-in-data'),
'no-confusing-v-for-v-if': require('./rules/no-confusing-v-for-v-if'),
'no-custom-modifiers-on-v-model': require('./rules/no-custom-modifiers-on-v-model'),
'no-deprecated-filter': require('./rules/no-deprecated-filter'),
Expand Down
105 changes: 105 additions & 0 deletions lib/rules/no-computed-in-data.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/**
* @fileoverview Ensure computed properties are not used in the data()
* @author IWANABETHATGUY
*/
'use strict'

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
const utils = require('../utils')
function topOfStack (stack) {
console.assert(Array.isArray(stack))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there a console.assert call here?

return stack[stack.length - 1]
}
module.exports = {
meta: {
type: 'problem',
docs: {
description: 'require computed properties are not used in the data()',
categories: ['strongly-recommended'],
IWANABETHATGUY marked this conversation as resolved.
Show resolved Hide resolved
recommended: false,
url: 'https://eslint.vuejs.org/rules/no-computed-in-data.html'
},
fixable: null, // or "code" or "whitespace"
schema: [
// fill in your schema
]
},

create: function (context) {
let dataAstNode
let targetObjectExpression
const memberExpressionMap = Object.create(null)
const scopeStack = []
// if data is a function, should reference its scope, other wise should be undefined
let dataPropertyScope
return {

ObjectExpression (node) {
if (!dataAstNode) {
dataAstNode = node.properties.find(
p =>
p.type === 'Property' &&
p.key.type === 'Identifier' &&
p.key.name === 'data'
)
if (dataAstNode) {
targetObjectExpression = node
if (dataAstNode.value.type === 'FunctionExpression') {
dataPropertyScope = dataAstNode.value
scopeStack.push(dataPropertyScope)
}
}
}
},
':function' (node) {
scopeStack.push(node)
},
':function:exit' (node) {
scopeStack.pop()
},
'Property:exit' (node) {
if (node === dataAstNode) {
dataAstNode = undefined
}
},
MemberExpression (node) {
if (dataAstNode && dataPropertyScope === topOfStack(scopeStack)) {
Copy link
Member

Choose a reason for hiding this comment

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

Vue components may not be at the top level.

export default {
  components: {
    // @vue/component
    MyComponent: {
      data() {
        return {
          bar: this.baz
        }
      },
      computed: {
        baz() {}
      }
    }
  },
}

// a memberExpression `like this.a.c.d` -> when traverse to c.d we can got the the full name -> this.a.c.d
const fullName = utils.parseMemberExpression(node).slice(0, 2).join('.')
if (memberExpressionMap[fullName]) {
// check if parent node in this array, if true ignore this node, such as `{a: this.a.c.d}`, this traverse function visit order is `this.a.c.d` -> `a.c.d` -> `c.d`
const hasParentNodeInArray = memberExpressionMap[fullName].some(nodeInMap => node.parent === nodeInMap)
Copy link
Member

Choose a reason for hiding this comment

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

I've found an issue with duplicate reports when the chain is deep. Perhaps checking the parent is a problem.

      <script>
      export default {
        data() {
          return {
            a: this.test.foo.bar.baz,
          }
        },
        computed: {
          test() {
            return {}
          },
        }
      }
      </script>

if (!hasParentNodeInArray) {
memberExpressionMap[fullName] = [...memberExpressionMap[fullName], node]
}
} else {
memberExpressionMap[fullName] = [node]
}
}
},
...utils.executeOnVue(context, obj => {
// check if targetObjectExpression is Vue component
if (targetObjectExpression !== obj) {
return
}
const computedPropertyNameList = utils.getComputedProperties(obj).map(item => `this.${item.key}`)
Object.keys(memberExpressionMap).forEach(fullName => {
IWANABETHATGUY marked this conversation as resolved.
Show resolved Hide resolved
const index = computedPropertyNameList.findIndex(name => fullName.startsWith(name))
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to use startsWith.

Suggested change
const index = computedPropertyNameList.findIndex(name => fullName.startsWith(name))
const index = computedPropertyNameList.findIndex(name => fullName === name)

if (index !== -1) {
memberExpressionMap[fullName].forEach(memberExpressionNode => {
context.report({
node: memberExpressionNode,
message: `Computed property '{{name}}' can't use in data property.`,
data: {
name: computedPropertyNameList[index]
}
})
})
}
})
})
}
}
}
2 changes: 1 addition & 1 deletion lib/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ module.exports = {
return computedPropertiesNode.value.properties
.filter(cp => cp.type === 'Property')
.map(cp => {
const key = cp.key.name
const key = getStaticPropertyName(cp)
let value
IWANABETHATGUY marked this conversation as resolved.
Show resolved Hide resolved

if (cp.value.type === 'FunctionExpression') {
Expand Down