[ESLint] Suggest moving inside a Hook or useCallback when bare function is a dependency

[gaearon]

This adds a new warning when a bare function is added as a dependency. It's always useless to do it because you might as well omit the deps array completely.

function handleChange(value) { // Needed as a dep, but it's a bare function so always invalidates
  setState(value);
}

useEffect(() => {
  return Store.subscribe(handleChange)
}, [handleChange]);

If the function is only used inside the Hook, it nudges you to move it inside it. That's usually an intuitive fix because then you can start fixing its deps.

If the function is used outside the Hook too, it suggests wrapping in useCallback. That lets you do "whack-a-mole" fixing up a chain of callbacks until you see what's causing things to change. There is an autofix for a simple case (variable declaration) where we know we wouldn't mess up hoisting.

This may be a good way to learn the dependencies mental model. However, there may be situations where just following suggestions gets you in a suggestion loop (e.g. circular function deps). I'm not sure I understand the limitations of this model well. However, even if you can't just follow autofixes, it would highlight the problem you'd have to discover on your own anyway. So at least this helps you see it. We may add more nudges (e.g. to use reducer) on top of this later to help you find the right fix.

[sizebot]

Details of bundled changes.

Comparing: 5d49daf...8ff6aaf

eslint-plugin-react-hooks

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
eslint-plugin-react-hooks.development.js	+6.9%	+6.9%	61.18 KB	65.39 KB	14.16 KB	15.13 KB	NODE_DEV
eslint-plugin-react-hooks.production.min.js	🔺+10.3%	🔺+9.6%	15.27 KB	16.83 KB	5.31 KB	5.82 KB	NODE_PROD
ESLintPluginReactHooks-dev.js	+7.2%	+6.8%	65.21 KB	69.9 KB	14.54 KB	15.52 KB	FB_WWW_DEV

Generated by 🚫 dangerJS

[gaearon]

I think this is a good compromise and helps learn the rules.