-
Notifications
You must be signed in to change notification settings - Fork 6
Update eslint-plugin-react-hooks to latest major release #37
Conversation
@@ -35,7 +35,7 @@ | |||
"eslint-plugin-import": "^2.14.0", | |||
"eslint-plugin-jsx-a11y": "^6.1.1", | |||
"eslint-plugin-react": "^7.12.2", | |||
"eslint-plugin-react-hooks": "^1.0.0" | |||
"eslint-plugin-react-hooks": "^2.0.1" |
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.
Are we major version bumping this package again?
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.
So before if you used hooks at top level; the app will crash. With pulling in this version now, if you use hooks at top level, you'll get a lint warning + app still crashes.
Based on that I'm leaning towards treating this as a minor release, however the react team considered this a breaking change.
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.
my vote is ship it as a patch release lol because I'd agree this seems like a breaking change :/
Curious why we don't use the full recommended config, namely we left off the rule:
This rule is pretty important and suggests fixes for common pitfalls involving stale hook dependencies. Commentary here |
@dev-cprice Added config for exhaustive-deps in this commit: 2173277 |
Summary
Update eslint-plugin-react-hooks to latest major release.
Related: https://github.com/facebook/react/pull/16528/files