Re-enable "react-hooks/exhaustive-deps" by default in wp-scripts linting

[emilio-martinez]

Is your feature request related to a problem? Please describe

I've relatively recently started using wp-scripts as part of developing blocks, and I noticed the react-hooks/exhaustive-deps rule is disabled.

Looking back through the history of PRs, it looks like it was added and then later removed while merging #14985 (comment). In the discussion, it's mentioned the rule was being too noisy. While that hasn't been my experience, to each their own; however, I do believe this rule helps catch easy mistakes in areas that can lead to large problems—side effects.

What does this rule do?

It will statically analyze the side effect callback to detect variables that should be included (if any) in useEffect's second argument, which enumerates dependencies to the effect in question. Additionally, it will be smart/aware of variables coming from other hooks. For example, it will recognize that the resulting value of useRef is mutable and therefore cannot be relied upon as a dependency.

useEffect's second argument is key to its behavior, not a stylistic choice. To the extent that this rule can help catch bugs, I believe it will help more that in will hurt in the long run.

Describe the solution you'd like

Similar to how the config currently extends plugin:react/recommended, I'd propose to add plugin:react-hooks/recommended to the extends array. This will add react-hooks/exhaustive-deps as a warning, not an error. Other common react configs follow this same kind of setup.

If agreeable, I'm happy to create a PR and aid with addressing any warnings that ensue from enabling this rule.

Describe alternatives you've considered

The workaround for this is for folks to create their own eslint config and modify as desired with wordpress/eslint-plugin/recommended as a starting point. This is definitely doable, although it seems like a roundabout step for something that's so widely used.

[gziolo]

@ZebulanStanphill, this issue sounds related to the root cause in your recent efforts to fix missing dependencies in useSelect 😃

@adamziel, do I recall correctly that you opened a similar issue in the past?

[adamziel]

@gziolo I did! #20936

tl;dr it would be beneficial but would also break a lot of existing hooks. I did not pursue it further, feel free to take over!

[talldan]

It was re-enabled in #24914.

There's a follow-up issue for solving the warnings - #47917.

I'll close this now as I think everything is tracked in that new issue.

[talldan]

Actually, I'm not entirely sure if it was re-enabled for the scripts package. Let me know if not and I can always re-open the issue.