-
Notifications
You must be signed in to change notification settings - Fork 18
Conversation
@lemonmade @GoodForOneFare @michelleyschen @TheMallen Can I get some initial feedback on this rule? Is this what we want to enforce? Anything I am missing? Worth noting that I am not looking to get this to the point where it will know everything about what you are returning. For example, if the variables you are returning are the result of a function call or from another file. |
I am board with this 👍 |
c3469da
to
4b55868
Compare
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.
Code LGTM but I think the flow control bits are probably going to prevent some violations from being caught
return; | ||
} | ||
|
||
inHook = false; |
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 suspect something like this will cause the rule to not complain, even though it should:
function useFoo() {
useEffect(() => {});
return [1, 2, 3, 4];
}
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.
Added some more tests to cover this https://github.com/Shopify/eslint-plugin-shopify/pull/237/files#diff-cde867597099df6969da77e4a149d359R206
closes #234