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 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
2 changes: 2 additions & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,8 @@ The following rules extend the rules provided by ESLint itself and apply them to
| [vue/key-spacing](./key-spacing.md) | enforce consistent spacing between keys and values in object literal properties | :wrench: |
| [vue/keyword-spacing](./keyword-spacing.md) | enforce consistent spacing before and after keywords | :wrench: |
| [vue/max-len](./max-len.md) | enforce a maximum line length | |
| [vue/no-boolean-default](./no-boolean-default.md) | disallow boolean defaults | :wrench: |
| [vue/no-computed-in-data](./no-computed-in-data.md) | disallow computed properties used in the data property | |
| [vue/no-empty-pattern](./no-empty-pattern.md) | disallow empty destructuring patterns | |
| [vue/no-extra-parens](./no-extra-parens.md) | disallow unnecessary parentheses | :wrench: |
| [vue/no-irregular-whitespace](./no-irregular-whitespace.md) | disallow irregular whitespace | |
Expand Down
55 changes: 55 additions & 0 deletions docs/rules/no-computed-in-data.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
pageClass: rule-details
sidebarDepth: 0
title: vue/no-computed-in-data
description: disallow computed properties used in the data property
---
# vue/no-computed-in-data
> disallow computed properties used in the data property

## :book: Rule Details

This rule report computed properties that used in data property

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

```vue
<script>
export default {
data() {
return {
/* ✓ GOOD */
value: 'hello ' + 'world',

/* ✗ BAD */
a: this.world,
b: this.world,
c: [this.world, this.number]
};
},
computed: {
world() {
return 'world';
},
number() {
return 1
}
},
};
</script>
```

</eslint-code-block>

## :wrench: Options

Nothing.

## :books: 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/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ module.exports = {
'no-async-in-computed-properties': require('./rules/no-async-in-computed-properties'),
'no-bare-strings-in-template': require('./rules/no-bare-strings-in-template'),
'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-data-object-declaration': require('./rules/no-deprecated-data-object-declaration'),
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: 'disallow computed properties used in the data property',
categories: undefined,
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]
}
})
})
}
})
})
}
}
}