Update eslint rule exhaustive deps to use new suggestions feature

[wdoug]

This pull request closes #16313

Assuming that eslint/eslint#12384 moves forward and adds the suggestions feature to eslint, this pull request updates the react-hooks/exhaustive-deps eslint rule to use the suggestions feature. This will result in the rule no-longer modifying code functionality when running eslint's autofixer while providing a new method to explicitly accept code suggestions for the rule.

For more details see: #16313

[codesandbox-ci]

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 b496ed7:

Sandbox	Source
intelligent-curie-0mjky	Configuration

[sizebot]

Details of bundled changes.

Comparing: 8e74a31...b496ed7

eslint-plugin-react-hooks

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
eslint-plugin-react-hooks.development.js	+0.6%	+0.4%	75.25 KB	75.67 KB	17.31 KB	17.37 KB	NODE_DEV
eslint-plugin-react-hooks.production.min.js	🔺+1.1%	🔺+0.6%	20.14 KB	20.37 KB	6.9 KB	6.94 KB	NODE_PROD

Size changes (stable)

Generated by 🚫 dangerJS against b496ed7

[sizebot]

Details of bundled changes.

Comparing: 8e74a31...b496ed7

eslint-plugin-react-hooks

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
eslint-plugin-react-hooks.development.js	+0.6%	+0.4%	75.26 KB	75.68 KB	17.31 KB	17.38 KB	NODE_DEV
eslint-plugin-react-hooks.production.min.js	🔺+1.1%	🔺+0.6%	20.16 KB	20.38 KB	6.91 KB	6.95 KB	NODE_PROD

Size changes (experimental)

Generated by 🚫 dangerJS against b496ed7

[gaearon]

Nice! Please ping this PR when it lands.

[wdoug]

Suggestions have been merged. Waiting on a release now.

[wdoug]

Suggestions released in 6.7.0

[gaearon]

What happens if you have older ESLint? Do we just lose the suggestions?

Should we only fix this forward?

[wdoug]

I was assuming that we would bump the major version of this package and possibly set the peer dependency of eslint to >=6.7.0. Probably adding some notes in the Readme would also be valuable.

I don't think we'd want to try to support both suggestions and the autofixer at the same time based on something like the eslint version because then the functionality would unexpectedly change when someone updated their version of eslint. Also, I don't think we want to support the autofix option going forward as it is ultimately unsafe.

I personally think it would be better to have no fixes for folks that happen to have an older version of eslint. For example, at my company, I enabled the exhaustive-deps rule and then immediately disabled it when I realized that most everyone at my company has their editors set to automatically run eslint's autofixer and we have a ton of misuses of hooks dependencies that, without manual work to address them, would cause user-facing bugs when this rule's autofix would run. Because of our current setup, it is likely that many of these bugs would end up getting out to production and affecting actual users. In our case, it would be much better to have the rule warning with no assistance fixing the issues than have the rule attempt to fix them for us without us noticing potential breaking changes.

[gaearon]

Just tried this with VS Code and couldn't get suggestions to show up. Does it mean extensions need to be updated?

[wdoug]

Yes. I have opened this issue for the vscode extension

[gaearon]

We probably shouldn’t merge until mainstream extensions catch up.

[wdoug]

Is there any chance we could release a beta prerelease with the suggestions feature while we wait for extensions to catch up?

[wdoug]

For reference, I'm hoping that this vscode-eslint pull request will get merged and released soon.

[gaearon]

I’d feel comfortable releasing after VSCode plugin update is live. Can you check if there’s anything holding it back on that end?

[gaearon]

Can you test that the rule works as expected with this release? microsoft/vscode-eslint#814 (comment)

[gaearon]

This PR is published as eslint-plugin-react-hooks@2.4.0-alpha.0. Please confirm that it works.

[gaearon]

Works in my testing.

[gaearon]

eslint-plugin-react-hooks@2.4.0 is out with this fix.

[gaearon]

This is great work. Thank you for doing it.

[wdoug]

My pleasure. It looks good on my end as well. Glad it finally got out.

[nstepien]

I prefered the rule with autofix working. Would it be possible to add an option to make it autofixable again?
The VSCode extension update with suggestions support isn't out yet, so it's making things a little bit awkward at the moment.

[LeopoldLerch]

I prefered the rule with autofix working. Would it be possible to add an option to make it autofixable again?
The VSCode extension update with suggestions support isn't out yet, so it's making things a little bit awkward at the moment.

Thing is, that is a common rule of eslint, that it does not change the code in regards of how it works. Which it in fact did with the autofix of the dependencies, expecially with useeffect. Altough a opt-in option for readding autofix could work, there is still the question if it SHOULD be there at all.

The update of the extension is already on the way, you can install it using @next . Regular version should be updated on the weekend as there corresponding thread of the vscode extension suggests.
In the meanwhile you can still use the prior-version of the eslint-ruleset.

[gaearon]

@nstepien You don't have to upgrade. :-) Feel free to revert to 2.3.0. We got this out early because #16313 has been causing pain for a while, and it can't wait more.