-
-
Notifications
You must be signed in to change notification settings - Fork 682
New Add vue/no-computed-in-data
Rule
#1075
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
Conversation
almost finish #958 |
@ota-meshi could please help for review? |
vue/no-computed-in-data
Rule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reply.
I have change requests.
</script> | ||
` | ||
}, | ||
// should not warn when objectExpression is not a vue component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | ||
const computedPropertyNameList = utils.getComputedProperties(obj).map(item => `this.${item.key}`) | ||
Object.keys(memberExpressionMap).forEach(fullName => { | ||
const index = computedPropertyNameList.findIndex(name => fullName.startsWith(name)) |
There was a problem hiding this comment.
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.
const index = computedPropertyNameList.findIndex(name => fullName.startsWith(name)) | |
const index = computedPropertyNameList.findIndex(name => fullName === name) |
} | ||
}, | ||
MemberExpression (node) { | ||
if (dataAstNode && dataPropertyScope === topOfStack(scopeStack)) { |
There was a problem hiding this comment.
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() {}
}
}
},
}
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) |
There was a problem hiding this comment.
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>
// ------------------------------------------------------------------------------ | ||
const utils = require('../utils') | ||
function topOfStack (stack) { | ||
console.assert(Array.isArray(stack)) |
There was a problem hiding this comment.
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?
This PR seems to be out of date, so I will close it. |
Fixes: #958