[WIP] Add ESLint rule for useEffect/useCallback/useMemo Hook dependencies

[gaearon]

This is just initial work from @calebmer. We didn't end up dogfooding this yet but @jamiebuilds expressed interest in picking it up. For some reason it doesn't pass tests — maybe there's some bug in the internal diff or maybe some dependency upgrade broke it. The semantics are also not set in stone and will probably need changes. Original description by @calebmer:

---

This diff adds a lint to enforce reactive dependencies are provided in the second argument of useEffect/useCallback. This lint is currently really strict. There are a lot of valid patterns it doesn't support. Unlike many other lints you should really be disabling this one when you want to do something more interesting then what the strict rule supports.

Here's what the lint does. The lint checks that identifiers and member expressions used in a "reactive hook callback" (lint internal name only) are declared in the reactive hook's dependency list.

function MyComponent(props) {
  useEffect(() => {
    console.log(props.foo);
  }, [props.foo]);
}

The lint requires that the dependency list be an array literal (no useEffect(() => {}, dependencies)) and that the dependency list not have spreads (no useEffect(() => {}, [...dependencies])). Both of these are valid forms in some interesting cases, though, but they make static analysis harder.

One thought is that perhaps we could support spreading an array as satisfying a dependency. For example allow:

function MyComponent(props) {
  useEffect(() => {
    for (const item of props.array) {
      console.log(item);
    }
  }, [...props.array]);
}

But I have not currently implemented this.

Some other reasonable cases (IMO) this lint does not allow include:

function MyComponent(props) {
  useEffect(() => {
    console.log(props.foo);
  }, [computeCacheKey(props.foo)]);
}

and:

function MyComponent(props) {
  useEffect(() => {
    console.log(props.foo);
  }, [props.foo.id]);
}

Both these forms allow you to be a bit more precise about the memoization breaking in useEffect and other reactive hooks.

This lint also warns about extra declarations in the dependency list:

function MyComponent(props) {
  useEffect(() => {
    console.log(props.foo);
  }, [props.foo, props.bar]);
}

And warns about duplicated declared dependencies:

function MyComponent(props) {
  useEffect(() => {
    console.log(props.foo);
  }, [props.foo, props.foo]);
}

Let me know if you think these constraints are reasonable. They might be good for 70-80% of cases and then after that there is always // eslint-disable-line.

[sizebot]

Details of bundled changes.

Comparing: 1ae3f29...6527d82

scheduler

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
scheduler.development.js	n/a	n/a	0 B	19.17 KB	0 B	5.74 KB	UMD_DEV
scheduler.production.min.js	n/a	n/a	0 B	3.16 KB	0 B	1.53 KB	UMD_PROD

eslint-plugin-react-hooks

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
eslint-plugin-react-hooks.development.js	+57.2%	+50.4%	25 KB	39.3 KB	5.75 KB	8.64 KB	NODE_DEV
eslint-plugin-react-hooks.production.min.js	🔺+79.4%	🔺+68.5%	4.83 KB	8.66 KB	1.77 KB	2.99 KB	NODE_PROD

Generated by 🚫 dangerJS

[gaearon]

#14052