-
-
Notifications
You must be signed in to change notification settings - Fork 205
Handle @computed
decorator without parentheses in no-side-effects
and require-computed-property-dependencies
rules
#800
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
Conversation
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.
FYI: There are several other rules that we should make this same fix to (either in the same PR or a separate PR is fine). If you search the tests for @computed(
then you'll see which rules currently support that decorator but probably don't support the decorator without parenthesis.
@@ -38,32 +38,38 @@ function isComputedPropertyBodyArg(node) { | |||
* @returns {ASTNode} function body of computed property | |||
*/ | |||
function getComputedPropertyFunctionBody(node) { |
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.
Can you add some unit tests for the new functionality in this function?
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 tried to add unit tests for this, but it relies on using node.parent
which didn't get populated when I ran babelEslint.parse(code)
.
@@ -38,32 +38,38 @@ function isComputedPropertyBodyArg(node) { | |||
* @returns {ASTNode} function body of computed property |
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.
Can you update the jsdoc comment with additional examples (@computed()
and @computed
), and mention the parameter can be a CallExpression or Identifier?
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 updated the doc for getComputedPropertyFunctionBody
, but not this one, because it seems to be only intended for when you're passing something in as the last argument to computed
.
} else if ( | ||
} | ||
|
||
if ( | ||
types.isDecorator(node.parent) && | ||
types.isMethodDefinition(node.parent.parent) && | ||
node.parent.parent.kind === 'get' | ||
) { | ||
// Example: @computed('first', 'last') get fullName() {} |
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.
Can you add a comment here with // Example: @computed get fullName() {}
?
@computed
decorator without parentheses in no-side-effects
and require-computed-property-dependencies
rules
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.
Thank you!
If you did
@computed
but omitted the parentheses, the linting wouldn't recognize it as a computed property, so it would be ignored. This effectively makes@computed
treated the same as@computed()
.