react-hooks-eslint-rule: add component props to pureScopes (#18051), … #19462
Conversation
…18051), update yarn.lock
| { | ||
| code: normalizeIndent` | ||
| function Example({ sortBy }) { | ||
| const foo = () => useMemo(() => console.log(sortBy), [sortBy]) |
There was a problem hiding this comment.
The thing that's odd about these examples is that they break the Rules of Hooks. Concretely, here we have a Hook call inside a callback. But Hook calls should only happen at the top level.
There was a problem hiding this comment.
Yes, it's totally right. So there is no bug but only a wrong hook usage.
There was a problem hiding this comment.
Given that, I think I could close this PR because it eventually fixes a situation caused by a wrong Hook usage.
Since Hook calls should only happen at the top level, component props will be alway in pureScopes array.
There was a problem hiding this comment.
@gaearon sorry for coming back to this topic, I have just one more question.
Is the use I made in this sandbox breaking the hook rule? Because I don't receive any message from rules-of-hooks and I am thinking it could be a legit use (even in a really contrived example) because the hook is called twice a render.
Sorry again if I am missing something.
There was a problem hiding this comment.
Totally misunderstood.
These examples are breaking the Rules of Hooks because the hook is called inside a function that is not a custom hook and neither in a JSX component.
The reason why the example in CodeSandbox is working is that the function within the hook is called begins with use. So...double error.
Sorry for the waste of time.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 65aaacf:
|
Details of bundled changes.Comparing: d29bf59...65aaacf eslint-plugin-react-hooks
Size changes (experimental) |
Details of bundled changes.Comparing: d29bf59...65aaacf eslint-plugin-react-hooks
Size changes (stable) |
…update yarn.lock
Summary
Following #18051 comment by an unexpected behavior is found related code like this:
An error message is thrown since
sortByis evaluated as an unnecessary dependency while it is a necessary dependency.According to me this was happening because
pureScopesarray did not contain the scope wheresortByis defined. I think this is a bug because component props (as well as argument received by a custom hook) should be contained bypureScopesarray.I changed the code in order to make these scopes always contained by
pureScopesarray.Test Plan
I have tested the desired behavior is obtained and even that Component
propsare defined as necessary only if they are used by the hook.