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
Eslint plugin react hooks #75
Conversation
eslintrc.json
Outdated
"react/react-in-jsx-scope": "error" | ||
"react/react-in-jsx-scope": "error", | ||
"react-hooks/rules-of-hooks": "error", | ||
"react-hooks/exhaustive-deps": "error" |
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 think that exhaustive-deps
could result in a lot of errors for many project, potentially it would be better to first release this as warning
and then switch to error
in the major version after that.
(that is how no-var
was rolled out: standard/standard#1586)
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.
Sounds good ! For some reason I thought the policy for standard eslint rules was no warnings, only errors.
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.
Done @LinusU
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.
LGTM 👍
Who decides when to merge, and when to release ? Or maybe even though it's been approved something else needs to be done ? |
Since I approved I think that anyone else from the team could merge if they also approve of this change 🚀 ping @voxpelli, would you mind looking at this? |
Regarding release, I don't see a problem with releasing this as a major version asap. However, it won't be part of |
I don't really have much to say about this one, I'm not much of a React user myself so I'm not really familiar with these rules :/ |
They're very standard (no pun intended !), they've been implemented in pretty much all rulesets (airbnb, create-react-app, react-hooks:recommended, next...) for a long time, and I was surprised to see it wasn't the case with eslint-config-standard-react, hence this PR. |
Just saw that @feross had approved one of the earlier PRs, let's merge 🚀 |
Since there are not other PRs/issues, and since there isn't much happening in this repo I'll cut a new major release right away 🚀 |
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix
[x] New feature
[ ] Other, please explain:
added eslint-plugin-react-hooks rules