New rule: no-attrs (resolves #52) #100
Conversation
lib/utils/utils.js
Outdated
| * @param {ASTNode} node The node to check. | ||
| * @returns {boolean} Whether or not the node is a ThisExpression with name of attrs. | ||
| */ | ||
| function isThisAttrsExpression(node) { |
There was a problem hiding this comment.
This is Ember specific stuff so maybe we should put it in utils/ember.js. And actually I would argue that we don't need to add this to utils at all, it will probably be used only once so we can simply keep it in the rule's file. What do you think @jbandura ?
There was a problem hiding this comment.
Hm, that's a good point actually. I moved it to the utils in order to write tests for this. On the other hand it's true that it might not be used outside of this rule. I can move it to the rule I think.
lib/rules/no-attrs-in-components.js
Outdated
|
|
||
| create(context) { | ||
| return { | ||
| MemberExpression(node) { |
There was a problem hiding this comment.
Although it's only being ported I'm thinking whether we should scope this check to component files only and do not throw error if it's used in util or anywhere else. What do you think? Anyway I'm ok with postponing this update at the moment.
There was a problem hiding this comment.
If this is not something that we already check, then I'd suggest postponing it. If it is, I'll add the check :)
There was a problem hiding this comment.
We have an util for checking that: https://github.com/ember-cli/eslint-plugin-ember/blob/master/lib/rules/order-in-components.js#L40
You should then add CallExpression:exit selector, store globally the found member expressions and then check if any of these exist inside that CallExpression. For the reference I wrote a similar check here: https://github.com/vuejs/eslint-plugin-vue/blob/master/lib/rules/no-side-effects-in-computed-properties.js#L39
This PR adds new rule regarding to #52