diff --git a/fixtures/eslint/.eslintrc.json b/fixtures/eslint/.eslintrc.json index 982d6c8eb65f8..2c6f6d2c0b6c8 100644 --- a/fixtures/eslint/.eslintrc.json +++ b/fixtures/eslint/.eslintrc.json @@ -9,6 +9,7 @@ }, "plugins": ["react-hooks"], "rules": { - "react-hooks/rules-of-hooks": 2 + "react-hooks/rules-of-hooks": 2, + "react-hooks/exhaustive-deps": 2 } } diff --git a/fixtures/eslint/index.js b/fixtures/eslint/index.js index 6fefccd55e67e..1052cac180fcc 100644 --- a/fixtures/eslint/index.js +++ b/fixtures/eslint/index.js @@ -4,8 +4,26 @@ // 2. "File > Add Folder to Workspace" this specific folder in VSCode with ESLint extension // 3. Changes to the rule source should get picked up without restarting ESLint server -function Foo() { - if (condition) { - useEffect(() => {}); - } +function Comment({comment, commentSource}) { + const currentUserID = comment.viewer.id; + const environment = RelayEnvironment.forUser(currentUserID); + const commentID = nullthrows(comment.id); + useEffect( + () => { + const subscription = SubscriptionCounter.subscribeOnce( + `StoreSubscription_${commentID}`, + () => + StoreSubscription.subscribe( + environment, + { + comment_id: commentID, + }, + currentUserID, + commentSource + ) + ); + return () => subscription.dispose(); + }, + [commentID, commentSource, currentUserID, environment] + ); } diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js new file mode 100644 index 0000000000000..a6d75435463b4 --- /dev/null +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -0,0 +1,1642 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +'use strict'; + +const ESLintTester = require('eslint').RuleTester; +const ReactHooksESLintPlugin = require('eslint-plugin-react-hooks'); +const ReactHooksESLintRule = ReactHooksESLintPlugin.rules['exhaustive-deps']; + +ESLintTester.setDefaultConfig({ + parser: 'babel-eslint', + parserOptions: { + ecmaVersion: 6, + sourceType: 'module', + }, +}); + +// *************************************************** +// For easier local testing, you can add to any case: +// { +// skip: true, +// --or-- +// only: true, +// ... +// } +// *************************************************** + +const tests = { + valid: [ + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }); + } + `, + }, + { + code: ` + function MyComponent() { + useEffect(() => { + const local = 42; + console.log(local); + }, []); + } + `, + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + }, + { + // OK because `props` wasn't defined. + // We don't technically know if `props` is supposed + // to be an import that hasn't been added yet, or + // a component-level variable. Ignore it until it + // gets defined (a different rule would flag it anyway). + code: ` + function MyComponent() { + useEffect(() => { + console.log(props.foo); + }, []); + } + `, + }, + { + code: ` + function MyComponent() { + const local1 = 42; + { + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }); + } + } + `, + }, + { + code: ` + function MyComponent() { + const local1 = 42; + { + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, [local1, local2]); + } + } + `, + }, + { + code: ` + function MyComponent() { + const local1 = 42; + function MyNestedComponent() { + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, [local2]); + } + } + `, + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + console.log(local); + }, [local]); + } + `, + }, + { + code: ` + function MyComponent() { + useEffect(() => { + console.log(unresolved); + }, []); + } + `, + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [,,,local,,,]); + } + `, + }, + { + code: ` + // Regression test + function MyComponent({ foo }) { + useEffect(() => { + console.log(foo.length); + }, [foo]); + } + `, + }, + { + code: ` + // Regression test + function MyComponent({ foo }) { + useEffect(() => { + console.log(foo.length); + console.log(foo.slice(0)); + }, [foo]); + } + `, + }, + { + code: ` + // Regression test + function MyComponent({ history }) { + useEffect(() => { + return history.listen(); + }, [history]); + } + `, + }, + { + // TODO: we might want to forbid dot-access in deps. + code: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + }, [props.foo]); + } + `, + }, + { + // TODO: we might want to forbid dot-access in deps. + code: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + }, [props.bar, props.foo]); + } + `, + }, + { + // TODO: we might want to forbid dot-access in deps. + code: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + }, [props.foo, props.bar]); + } + `, + }, + { + // TODO: we might want to forbid dot-access in deps. + code: ` + function MyComponent(props) { + const local = 42; + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + console.log(local); + }, [props.foo, props.bar, local]); + } + `, + }, + { + // TODO: we might want to warn "props.foo" + // is extraneous because we already have "props". + code: ` + function MyComponent(props) { + const local = 42; + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + }, [props, props.foo]); + } + `, + }, + { + code: ` + function MyComponent(props) { + useCustomEffect(() => { + console.log(props.foo); + }); + } + `, + options: [{additionalHooks: 'useCustomEffect'}], + }, + { + // TODO: we might want to forbid dot-access in deps. + code: ` + function MyComponent(props) { + useCustomEffect(() => { + console.log(props.foo); + }, [props.foo]); + } + `, + options: [{additionalHooks: 'useCustomEffect'}], + }, + { + code: ` + // Valid because we don't care about hooks outside of components. + const local = 42; + useEffect(() => { + console.log(local); + }, []); + `, + }, + { + code: ` + // Valid because we don't care about hooks outside of components. + const local1 = 42; + { + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, []); + } + `, + }, + { + code: ` + function MyComponent() { + const ref = useRef(); + useEffect(() => { + console.log(ref.current); + }, [ref]); + } + `, + }, + { + code: ` + function MyComponent() { + const ref = useRef(); + useEffect(() => { + console.log(ref.current); + }, []); + } + `, + }, + { + code: ` + function MyComponent({ maybeRef2 }) { + const definitelyRef1 = useRef(); + const definitelyRef2 = useRef(); + const maybeRef1 = useSomeOtherRefyThing(); + const [state1, setState1] = useState(); + const [state2, setState2] = React.useState(); + const [state3, dispatch1] = useReducer(); + const [state4, dispatch2] = React.useReducer(); + const [state5, maybeSetState] = useFunnyState(); + const [state6, maybeDispatch] = useFunnyReducer(); + function mySetState() {} + function myDispatch() {} + + useEffect(() => { + // Known to be static + console.log(definitelyRef1.current); + console.log(definitelyRef2.current); + console.log(maybeRef1.current); + console.log(maybeRef2.current); + setState1(); + setState2(); + dispatch1(); + dispatch2(); + + // Dynamic + console.log(state1); + console.log(state2); + console.log(state3); + console.log(state4); + console.log(state5); + console.log(state6); + mySetState(); + myDispatch(); + + // Not sure; assume dynamic + maybeSetState(); + maybeDispatch(); + }, [ + // Dynamic + state1, state2, state3, state4, state5, state6, + maybeRef1, maybeRef2, + + // Not sure; assume dynamic + mySetState, myDispatch, + maybeSetState, maybeDispatch + + // In this test, we don't specify static deps. + // That should be okay. + ]); + } + `, + }, + { + code: ` + function MyComponent({ maybeRef2 }) { + const definitelyRef1 = useRef(); + const definitelyRef2 = useRef(); + const maybeRef1 = useSomeOtherRefyThing(); + + const [state1, setState1] = useState(); + const [state2, setState2] = React.useState(); + const [state3, dispatch1] = useReducer(); + const [state4, dispatch2] = React.useReducer(); + + const [state5, maybeSetState] = useFunnyState(); + const [state6, maybeDispatch] = useFunnyReducer(); + + function mySetState() {} + function myDispatch() {} + + useEffect(() => { + // Known to be static + console.log(definitelyRef1.current); + console.log(definitelyRef2.current); + console.log(maybeRef1.current); + console.log(maybeRef2.current); + setState1(); + setState2(); + dispatch1(); + dispatch2(); + + // Dynamic + console.log(state1); + console.log(state2); + console.log(state3); + console.log(state4); + console.log(state5); + console.log(state6); + mySetState(); + myDispatch(); + + // Not sure; assume dynamic + maybeSetState(); + maybeDispatch(); + }, [ + // Dynamic + state1, state2, state3, state4, state5, state6, + maybeRef1, maybeRef2, + + // Not sure; assume dynamic + mySetState, myDispatch, + maybeSetState, maybeDispatch, + + // In this test, we specify static deps. + // That should be okay too! + definitelyRef1, definitelyRef2, setState1, setState2, dispatch1, dispatch2 + ]); + } + `, + }, + { + code: ` + const MyComponent = forwardRef((props, ref) => { + useImperativeHandle(ref, () => ({ + focus() { + alert(props.hello); + } + })) + }); + `, + }, + { + code: ` + const MyComponent = forwardRef((props, ref) => { + useImperativeHandle(ref, () => ({ + focus() { + alert(props.hello); + } + }), [props.hello]) + }); + `, + }, + ], + invalid: [ + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, []); + } + `, + output: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + // Regression test + function MyComponent() { + const local = 42; + useEffect(() => { + if (true) { + console.log(local); + } + }, []); + } + `, + output: ` + // Regression test + function MyComponent() { + const local = 42; + useEffect(() => { + if (true) { + console.log(local); + } + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + // Regression test + function MyComponent() { + const local = 42; + useEffect(() => { + try { + console.log(local); + } finally {} + }, []); + } + `, + output: ` + // Regression test + function MyComponent() { + const local = 42; + useEffect(() => { + try { + console.log(local); + } finally {} + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + // Regression test + function MyComponent() { + const local = 42; + useEffect(() => { + function inner() { + console.log(local); + } + inner(); + }, []); + } + `, + output: ` + // Regression test + function MyComponent() { + const local = 42; + useEffect(() => { + function inner() { + console.log(local); + } + inner(); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local1 = 42; + { + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, []); + } + } + `, + output: ` + function MyComponent() { + const local1 = 42; + { + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, [local1, local2]); + } + } + `, + errors: [ + "React Hook useEffect has missing dependencies: 'local1' and 'local2'. " + + 'Either include them or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local1 = 42; + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, [local1]); + } + `, + output: ` + function MyComponent() { + const local1 = 42; + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, [local1, local2]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local2'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local1 = 42; + const local2 = 42; + useEffect(() => { + console.log(local1); + }, [local1, local2]); + } + `, + output: ` + function MyComponent() { + const local1 = 42; + const local2 = 42; + useEffect(() => { + console.log(local1); + }, [local1]); + } + `, + errors: [ + "React Hook useEffect has an unnecessary dependency: 'local2'. " + + 'Either exclude it or remove the dependency array.', + ], + }, + { + // TODO: this case is weird. + // Maybe it should not consider local1 unused despite component nesting? + code: ` + function MyComponent() { + const local1 = 42; + function MyNestedComponent() { + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, [local1]); + } + } + `, + output: ` + function MyComponent() { + const local1 = 42; + function MyNestedComponent() { + const local2 = 42; + useEffect(() => { + console.log(local1); + console.log(local2); + }, [local2]); + } + } + `, + errors: [ + // Focus on the more important part first (missing dep) + "React Hook useEffect has a missing dependency: 'local2'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + console.log(local); + }, []); + } + `, + output: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + console.log(local); + }, [local, local]); + } + `, + output: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a duplicate dependency: 'local'. " + + 'Either omit it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + useEffect(() => {}, [local]); + } + `, + output: ` + function MyComponent() { + useEffect(() => {}, []); + } + `, + errors: [ + "React Hook useEffect has an unnecessary dependency: 'local'. " + + 'Either exclude it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent({ history }) { + useEffect(() => { + return history.listen(); + }, []); + } + `, + output: ` + function MyComponent({ history }) { + useEffect(() => { + return history.listen(); + }, [history]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'history'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent({ history }) { + useEffect(() => { + return [ + history.foo.bar[2].dobedo.listen(), + history.foo.bar().dobedo.listen[2] + ]; + }, []); + } + `, + output: ` + function MyComponent({ history }) { + useEffect(() => { + return [ + history.foo.bar[2].dobedo.listen(), + history.foo.bar().dobedo.listen[2] + ]; + }, [history.foo]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'history.foo'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const dependencies = []; + useEffect(() => {}, dependencies); + } + `, + output: ` + function MyComponent() { + const dependencies = []; + useEffect(() => {}, dependencies); + } + `, + errors: [ + 'React Hook useEffect has a second argument which is not an array ' + + "literal. This means we can't statically verify whether you've " + + 'passed the correct dependencies.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + const dependencies = [local]; + useEffect(() => { + console.log(local); + }, dependencies); + } + `, + // TODO: should this autofix or bail out? + output: ` + function MyComponent() { + const local = 42; + const dependencies = [local]; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + errors: [ + 'React Hook useEffect has a second argument which is not an array ' + + "literal. This means we can't statically verify whether you've " + + 'passed the correct dependencies.', + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + const dependencies = [local]; + useEffect(() => { + console.log(local); + }, [...dependencies]); + } + `, + // TODO: should this autofix or bail out? + output: ` + function MyComponent() { + const local = 42; + const dependencies = [local]; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + 'React Hook useEffect has a spread element in its dependency array. ' + + "This means we can't statically verify whether you've passed the " + + 'correct dependencies.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local, ...dependencies]); + } + `, + output: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local, ...dependencies]); + } + `, + errors: [ + 'React Hook useEffect has a spread element in its dependency array. ' + + "This means we can't statically verify whether you've passed the " + + 'correct dependencies.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [computeCacheKey(local)]); + } + `, + // TODO: I'm not sure this is a good idea. + // Maybe bail out? + output: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + 'React Hook useEffect has a complex expression in the dependency array. ' + + 'Extract it to a separate variable so it can be statically checked.', + ], + }, + { + // only: true, + code: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.items[0]); + }, [props.items[0]]); + } + `, + output: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.items[0]); + }, [props.items]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'props.items'. " + + 'Either include it or remove the dependency array.', + 'React Hook useEffect has a complex expression in the dependency array. ' + + 'Extract it to a separate variable so it can be statically checked.', + ], + }, + { + code: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.items[0]); + }, [props.items, props.items[0]]); + } + `, + // TODO: ideally autofix would remove the bad expression? + output: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.items[0]); + }, [props.items, props.items[0]]); + } + `, + errors: [ + 'React Hook useEffect has a complex expression in the dependency array. ' + + 'Extract it to a separate variable so it can be statically checked.', + ], + }, + { + code: ` + function MyComponent({ items }) { + useEffect(() => { + console.log(items[0]); + }, [items[0]]); + } + `, + output: ` + function MyComponent({ items }) { + useEffect(() => { + console.log(items[0]); + }, [items]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'items'. " + + 'Either include it or remove the dependency array.', + 'React Hook useEffect has a complex expression in the dependency array. ' + + 'Extract it to a separate variable so it can be statically checked.', + ], + }, + { + code: ` + function MyComponent({ items }) { + useEffect(() => { + console.log(items[0]); + }, [items, items[0]]); + } + `, + // TODO: ideally autofix would remove the bad expression? + output: ` + function MyComponent({ items }) { + useEffect(() => { + console.log(items[0]); + }, [items, items[0]]); + } + `, + errors: [ + 'React Hook useEffect has a complex expression in the dependency array. ' + + 'Extract it to a separate variable so it can be statically checked.', + ], + }, + { + // TODO: need to think more about this case. + code: ` + function MyComponent() { + const local = {id: 42}; + useEffect(() => { + console.log(local); + }, [local.id]); + } + `, + // TODO: this may not be a good idea. + output: ` + function MyComponent() { + const local = {id: 42}; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local, local]); + } + `, + output: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a duplicate dependency: 'local'. " + + 'Either omit it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local1 = 42; + useEffect(() => { + const local1 = 42; + console.log(local1); + }, [local1]); + } + `, + output: ` + function MyComponent() { + const local1 = 42; + useEffect(() => { + const local1 = 42; + console.log(local1); + }, []); + } + `, + errors: [ + "React Hook useEffect has an unnecessary dependency: 'local1'. " + + 'Either exclude it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local1 = 42; + useEffect(() => {}, [local1]); + } + `, + output: ` + function MyComponent() { + const local1 = 42; + useEffect(() => {}, []); + } + `, + errors: [ + "React Hook useEffect has an unnecessary dependency: 'local1'. " + + 'Either exclude it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + }, []); + } + `, + // TODO: we need to think about ideal output here. + // Should it capture by default? + output: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + }, [props.foo]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + }, []); + } + `, + // TODO: we need to think about ideal output here. + // Should it capture by default? + output: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + }, [props.bar, props.foo]); + } + `, + errors: [ + "React Hook useEffect has missing dependencies: 'props.bar' and 'props.foo'. " + + 'Either include them or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + let a, b, c, d, e, f, g; + useEffect(() => { + console.log(b, e, d, c, a, g, f); + }, [c, a, g]); + } + `, + // Don't alphabetize if it wasn't alphabetized in the first place. + output: ` + function MyComponent(props) { + let a, b, c, d, e, f, g; + useEffect(() => { + console.log(b, e, d, c, a, g, f); + }, [c, a, g, b, e, d, f]); + } + `, + errors: [ + "React Hook useEffect has missing dependencies: 'b', 'd', 'e', and 'f'. " + + 'Either include them or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + let a, b, c, d, e, f, g; + useEffect(() => { + console.log(b, e, d, c, a, g, f); + }, [a, c, g]); + } + `, + // Alphabetize if it was alphabetized. + output: ` + function MyComponent(props) { + let a, b, c, d, e, f, g; + useEffect(() => { + console.log(b, e, d, c, a, g, f); + }, [a, b, c, d, e, f, g]); + } + `, + errors: [ + "React Hook useEffect has missing dependencies: 'b', 'd', 'e', and 'f'. " + + 'Either include them or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + let a, b, c, d, e, f, g; + useEffect(() => { + console.log(b, e, d, c, a, g, f); + }, []); + } + `, + // Alphabetize if it was empty. + output: ` + function MyComponent(props) { + let a, b, c, d, e, f, g; + useEffect(() => { + console.log(b, e, d, c, a, g, f); + }, [a, b, c, d, e, f, g]); + } + `, + errors: [ + "React Hook useEffect has missing dependencies: 'a', 'b', 'c', 'd', 'e', 'f', and 'g'. " + + 'Either include them or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + const local = 42; + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + console.log(local); + }, []); + } + `, + // TODO: we need to think about ideal output here. + // Should it capture by default? + output: ` + function MyComponent(props) { + const local = 42; + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + console.log(local); + }, [local, props.bar, props.foo]); + } + `, + errors: [ + "React Hook useEffect has missing dependencies: 'local', 'props.bar', and 'props.foo'. " + + 'Either include them or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + const local = 42; + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + console.log(local); + }, [props]); + } + `, + output: ` + function MyComponent(props) { + const local = 42; + useEffect(() => { + console.log(props.foo); + console.log(props.bar); + console.log(local); + }, [local, props]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + }, []); + useCallback(() => { + console.log(props.foo); + }, []); + useMemo(() => { + console.log(props.foo); + }, []); + React.useEffect(() => { + console.log(props.foo); + }, []); + React.useCallback(() => { + console.log(props.foo); + }, []); + React.useMemo(() => { + console.log(props.foo); + }, []); + React.notReactiveHook(() => { + console.log(props.foo); + }, []); + } + `, + output: ` + function MyComponent(props) { + useEffect(() => { + console.log(props.foo); + }, [props.foo]); + useCallback(() => { + console.log(props.foo); + }, [props.foo]); + useMemo(() => { + console.log(props.foo); + }, [props.foo]); + React.useEffect(() => { + console.log(props.foo); + }, [props.foo]); + React.useCallback(() => { + console.log(props.foo); + }, [props.foo]); + React.useMemo(() => { + console.log(props.foo); + }, [props.foo]); + React.notReactiveHook(() => { + console.log(props.foo); + }, []); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + "React Hook useCallback has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + "React Hook useMemo has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + "React Hook React.useEffect has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + "React Hook React.useCallback has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + "React Hook React.useMemo has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + useCustomEffect(() => { + console.log(props.foo); + }, []); + useEffect(() => { + console.log(props.foo); + }, []); + React.useEffect(() => { + console.log(props.foo); + }, []); + React.useCustomEffect(() => { + console.log(props.foo); + }, []); + } + `, + output: ` + function MyComponent(props) { + useCustomEffect(() => { + console.log(props.foo); + }, [props.foo]); + useEffect(() => { + console.log(props.foo); + }, [props.foo]); + React.useEffect(() => { + console.log(props.foo); + }, [props.foo]); + React.useCustomEffect(() => { + console.log(props.foo); + }, []); + } + `, + options: [{additionalHooks: 'useCustomEffect'}], + errors: [ + "React Hook useCustomEffect has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + "React Hook useEffect has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + "React Hook React.useEffect has a missing dependency: 'props.foo'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [a ? local : b]); + } + `, + // TODO: should we bail out instead? + output: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + 'React Hook useEffect has a complex expression in the dependency array. ' + + 'Extract it to a separate variable so it can be statically checked.', + ], + }, + { + code: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [a && local]); + } + `, + // TODO: should we bail out instead? + output: ` + function MyComponent() { + const local = 42; + useEffect(() => { + console.log(local); + }, [local]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'local'. " + + 'Either include it or remove the dependency array.', + 'React Hook useEffect has a complex expression in the dependency array. ' + + 'Extract it to a separate variable so it can be statically checked.', + ], + }, + { + code: ` + function MyComponent() { + const ref = useRef(); + const [state, setState] = useState(); + useEffect(() => { + ref.current = 42; + setState(state + 1); + }, []); + } + `, + output: ` + function MyComponent() { + const ref = useRef(); + const [state, setState] = useState(); + useEffect(() => { + ref.current = 42; + setState(state + 1); + }, [state]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'state'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent() { + const ref = useRef(); + const [state, setState] = useState(); + useEffect(() => { + ref.current = 42; + setState(state + 1); + }, [ref]); + } + `, + // We don't ask to remove static deps but don't add them either. + // Don't suggest removing "ref" (it's fine either way) + // but *do* add "state". *Don't* add "setState" ourselves. + output: ` + function MyComponent() { + const ref = useRef(); + const [state, setState] = useState(); + useEffect(() => { + ref.current = 42; + setState(state + 1); + }, [ref, state]); + } + `, + errors: [ + "React Hook useEffect has a missing dependency: 'state'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + const ref1 = useRef(); + const ref2 = useRef(); + useEffect(() => { + ref1.current.focus(); + console.log(ref2.current.textContent); + alert(props.someOtherRefs.current.innerHTML); + fetch(props.color); + }, []); + } + `, + output: ` + function MyComponent(props) { + const ref1 = useRef(); + const ref2 = useRef(); + useEffect(() => { + ref1.current.focus(); + console.log(ref2.current.textContent); + alert(props.someOtherRefs.current.innerHTML); + fetch(props.color); + }, [props.color, props.someOtherRefs]); + } + `, + errors: [ + "React Hook useEffect has missing dependencies: 'props.color' and 'props.someOtherRefs'. " + + 'Either include them or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + const ref1 = useRef(); + const ref2 = useRef(); + useEffect(() => { + ref1.current.focus(); + console.log(ref2.current.textContent); + alert(props.someOtherRefs.current.innerHTML); + fetch(props.color); + }, [ref1.current, ref2.current, props.someOtherRefs, props.color]); + } + `, + output: ` + function MyComponent(props) { + const ref1 = useRef(); + const ref2 = useRef(); + useEffect(() => { + ref1.current.focus(); + console.log(ref2.current.textContent); + alert(props.someOtherRefs.current.innerHTML); + fetch(props.color); + }, [props.someOtherRefs, props.color]); + } + `, + errors: [ + "React Hook useEffect has unnecessary dependencies: 'ref1.current' and 'ref2.current'. " + + 'Either exclude them or remove the dependency array. ' + + "Mutable values like 'ref1.current' aren't valid dependencies " + + "because their mutation doesn't re-render the component.", + ], + }, + { + code: ` + function MyComponent() { + const ref = useRef(); + useEffect(() => { + console.log(ref.current); + }, [ref.current]); + } + `, + output: ` + function MyComponent() { + const ref = useRef(); + useEffect(() => { + console.log(ref.current); + }, []); + } + `, + errors: [ + "React Hook useEffect has an unnecessary dependency: 'ref.current'. " + + 'Either exclude it or remove the dependency array. ' + + "Mutable values like 'ref.current' aren't valid dependencies " + + "because their mutation doesn't re-render the component.", + ], + }, + { + code: ` + function MyComponent() { + const ref = useRef(); + useEffect(() => { + console.log(ref.current); + }, [ref.current, ref]); + } + `, + output: ` + function MyComponent() { + const ref = useRef(); + useEffect(() => { + console.log(ref.current); + }, [ref]); + } + `, + errors: [ + "React Hook useEffect has an unnecessary dependency: 'ref.current'. " + + 'Either exclude it or remove the dependency array. ' + + "Mutable values like 'ref.current' aren't valid dependencies " + + "because their mutation doesn't re-render the component.", + ], + }, + { + code: ` + const MyComponent = forwardRef((props, ref) => { + useImperativeHandle(ref, () => ({ + focus() { + alert(props.hello); + } + }), []) + }); + `, + output: ` + const MyComponent = forwardRef((props, ref) => { + useImperativeHandle(ref, () => ({ + focus() { + alert(props.hello); + } + }), [props.hello]) + }); + `, + errors: [ + "React Hook useImperativeHandle has a missing dependency: 'props.hello'. " + + 'Either include it or remove the dependency array.', + ], + }, + { + code: ` + function MyComponent(props) { + useEffect(() => { + if (props.onChange) { + props.onChange(); + } + }, []); + } + `, + // TODO: [props.onChange] is superfluous. Fix to just [props.onChange]. + output: ` + function MyComponent(props) { + useEffect(() => { + if (props.onChange) { + props.onChange(); + } + }, [props, props.onChange]); + } + `, + errors: [ + // TODO: reporting props separately is superfluous. Fix to just props.onChange. + "React Hook useEffect has missing dependencies: 'props' and 'props.onChange'. " + + 'Either include them or remove the dependency array.', + ], + }, + ], +}; + +// For easier local testing +if (!process.env.CI) { + let only = []; + let skipped = []; + [...tests.valid, ...tests.invalid].forEach(t => { + if (t.skip) { + delete t.skip; + skipped.push(t); + } + if (t.only) { + delete t.only; + only.push(t); + } + }); + const predicate = t => { + if (only.length > 0) { + return only.indexOf(t) !== -1; + } + if (skipped.length > 0) { + return skipped.indexOf(t) === -1; + } + return true; + }; + tests.valid = tests.valid.filter(predicate); + tests.invalid = tests.invalid.filter(predicate); +} + +const eslintTester = new ESLintTester(); +eslintTester.run('react-hooks', ReactHooksESLintRule, tests); diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js new file mode 100644 index 0000000000000..34304c0c91e54 --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -0,0 +1,567 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/* eslint-disable no-for-of-loops/no-for-of-loops */ + +'use strict'; + +// const [state, setState] = useState() / React.useState() +// ^^^ true for this reference +// const [state, dispatch] = useReducer() / React.useReducer() +// ^^^ true for this reference +// const ref = useRef() +// ^^ true for this reference +// False for everything else. +function isDefinitelyStaticDependency(reference) { + // This function is written defensively because I'm not sure about corner cases. + // TODO: we can strengthen this if we're sure about the types. + const resolved = reference.resolved; + if (resolved == null || !Array.isArray(resolved.defs)) { + return false; + } + const def = resolved.defs[0]; + if (def == null || def.node.init == null) { + return false; + } + // Look for `let stuff = SomeHook();` + const init = def.node.init; + if (init.callee == null) { + return false; + } + let callee = init.callee; + // Step into `= React.something` initializer. + if ( + callee.type === 'MemberExpression' && + callee.object.name === 'React' && + callee.property != null && + !callee.computed + ) { + callee = callee.property; + } + if (callee.type !== 'Identifier') { + return; + } + const id = def.node.id; + if (callee.name === 'useRef' && id.type === 'Identifier') { + // useRef() return value is static. + return true; + } else if (callee.name === 'useState' || callee.name === 'useReducer') { + // Only consider second value in initializing tuple static. + if ( + id.type === 'ArrayPattern' && + id.elements.length === 2 && + Array.isArray(reference.resolved.identifiers) && + // Is second tuple value the same reference we're checking? + id.elements[1] === reference.resolved.identifiers[0] + ) { + return true; + } + } + // By default assume it's dynamic. + return false; +} + +export default { + meta: { + fixable: 'code', + schema: [ + { + type: 'object', + additionalProperties: false, + properties: { + additionalHooks: { + type: 'string', + }, + }, + }, + ], + }, + create(context) { + // Parse the `additionalHooks` regex. + const additionalHooks = + context.options && + context.options[0] && + context.options[0].additionalHooks + ? new RegExp(context.options[0].additionalHooks) + : undefined; + const options = {additionalHooks}; + + return { + FunctionExpression: visitFunctionExpression, + ArrowFunctionExpression: visitFunctionExpression, + }; + + /** + * Visitor for both function expressions and arrow function expressions. + */ + function visitFunctionExpression(node) { + // We only want to lint nodes which are reactive hook callbacks. + if ( + (node.type !== 'FunctionExpression' && + node.type !== 'ArrowFunctionExpression') || + node.parent.type !== 'CallExpression' + ) { + return; + } + + const callbackIndex = getReactiveHookCallbackIndex( + node.parent.callee, + options, + ); + if (node.parent.arguments[callbackIndex] !== node) { + return; + } + + // Get the reactive hook node. + const reactiveHook = node.parent.callee; + + // Get the declared dependencies for this reactive hook. If there is no + // second argument then the reactive callback will re-run on every render. + // So no need to check for dependency inclusion. + const depsIndex = callbackIndex + 1; + const declaredDependenciesNode = node.parent.arguments[depsIndex]; + if (!declaredDependenciesNode) { + return; + } + + // Get the current scope. + const scope = context.getScope(); + + // Find all our "pure scopes". On every re-render of a component these + // pure scopes may have changes to the variables declared within. So all + // variables used in our reactive hook callback but declared in a pure + // scope need to be listed as dependencies of our reactive hook callback. + // + // According to the rules of React you can't read a mutable value in pure + // scope. We can't enforce this in a lint so we trust that all variables + // declared outside of pure scope are indeed frozen. + const pureScopes = new Set(); + { + let currentScope = scope.upper; + while (currentScope) { + pureScopes.add(currentScope); + if (currentScope.type === 'function') { + break; + } + currentScope = currentScope.upper; + } + // If there is no parent function scope then there are no pure scopes. + // The ones we've collected so far are incorrect. So don't continue with + // the lint. + if (!currentScope) { + return; + } + } + + // Get dependencies from all our resolved references in pure scopes. + // Key is dependency string, value is whether it's static. + const dependencies = new Map(); + gatherDependenciesRecursively(scope); + + function gatherDependenciesRecursively(currentScope) { + for (const reference of currentScope.references) { + // If this reference is not resolved or it is not declared in a pure + // scope then we don't care about this reference. + if (!reference.resolved) { + continue; + } + if (!pureScopes.has(reference.resolved.scope)) { + continue; + } + // Narrow the scope of a dependency if it is, say, a member expression. + // Then normalize the narrowed dependency. + + const referenceNode = fastFindReferenceWithParent( + node, + reference.identifier, + ); + const dependencyNode = getDependency(referenceNode); + const dependency = toPropertyAccessString(dependencyNode); + + // Add the dependency to a map so we can make sure it is referenced + // again in our dependencies array. Remember whether it's static. + if (!dependencies.has(dependency)) { + const isStatic = isDefinitelyStaticDependency(reference); + dependencies.set(dependency, isStatic); + } + } + for (const childScope of currentScope.childScopes) { + gatherDependenciesRecursively(childScope); + } + } + + const declaredDependencies = []; + if (declaredDependenciesNode.type !== 'ArrayExpression') { + // If the declared dependencies are not an array expression then we + // can't verify that the user provided the correct dependencies. Tell + // the user this in an error. + context.report({ + node: declaredDependenciesNode, + message: + `React Hook ${context.getSource(reactiveHook)} has a second ` + + "argument which is not an array literal. This means we can't " + + "statically verify whether you've passed the correct dependencies.", + }); + } else { + declaredDependenciesNode.elements.forEach(declaredDependencyNode => { + // Skip elided elements. + if (declaredDependencyNode === null) { + return; + } + // If we see a spread element then add a special warning. + if (declaredDependencyNode.type === 'SpreadElement') { + context.report({ + node: declaredDependencyNode, + message: + `React Hook ${context.getSource(reactiveHook)} has a spread ` + + "element in its dependency array. This means we can't " + + "statically verify whether you've passed the " + + 'correct dependencies.', + }); + return; + } + // Try to normalize the declared dependency. If we can't then an error + // will be thrown. We will catch that error and report an error. + let declaredDependency; + try { + declaredDependency = toPropertyAccessString(declaredDependencyNode); + } catch (error) { + if (/Unsupported node type/.test(error.message)) { + context.report({ + node: declaredDependencyNode, + message: + `React Hook ${context.getSource(reactiveHook)} has a ` + + `complex expression in the dependency array. ` + + 'Extract it to a separate variable so it can be statically checked.', + }); + return; + } else { + throw error; + } + } + // Add the dependency to our declared dependency map. + declaredDependencies.push({ + key: declaredDependency, + node: declaredDependencyNode, + }); + }); + } + + // TODO: we can do a pass at this code and pick more appropriate + // data structures to avoid nested loops if we can. + let suggestedDependencies = []; + let duplicateDependencies = new Set(); + let unnecessaryDependencies = new Set(); + let missingDependencies = new Set(); + let actualDependencies = Array.from(dependencies.keys()); + + function satisfies(actualDep, dep) { + return actualDep === dep || actualDep.startsWith(dep + '.'); + } + + // First, ensure what user specified makes sense. + declaredDependencies.forEach(({key}) => { + if (actualDependencies.some(actualDep => satisfies(actualDep, key))) { + // Legit dependency. + if (suggestedDependencies.indexOf(key) === -1) { + suggestedDependencies.push(key); + } else { + // Duplicate. Do nothing. + duplicateDependencies.add(key); + } + } else { + // Unnecessary dependency. Do nothing. + unnecessaryDependencies.add(key); + } + }); + + // Then fill in the missing ones. + dependencies.forEach((isStatic, key) => { + if ( + !suggestedDependencies.some(suggestedDep => + satisfies(key, suggestedDep), + ) + ) { + if (!isStatic) { + // Legit missing. + suggestedDependencies.push(key); + missingDependencies.add(key); + } + } else { + // Already did that. Do nothing. + } + }); + + function areDeclaredDepsAlphabetized() { + if (declaredDependencies.length === 0) { + return true; + } + const declaredDepKeys = declaredDependencies.map(dep => dep.key); + const sortedDeclaredDepKeys = declaredDepKeys.slice().sort(); + return declaredDepKeys.join(',') === sortedDeclaredDepKeys.join(','); + } + + if (areDeclaredDepsAlphabetized()) { + // Alphabetize the autofix, but only if deps were already alphabetized. + suggestedDependencies.sort(); + } + + const problemCount = + duplicateDependencies.size + + missingDependencies.size + + unnecessaryDependencies.size; + + if (problemCount === 0) { + return; + } + + function getWarningMessage(deps, singlePrefix, label, fixVerb) { + if (deps.size === 0) { + return null; + } + return ( + (deps.size > 1 ? '' : singlePrefix + ' ') + + label + + ' ' + + (deps.size > 1 ? 'dependencies' : 'dependency') + + ': ' + + joinEnglish( + Array.from(deps) + .sort() + .map(name => "'" + name + "'"), + ) + + `. Either ${fixVerb} ${ + deps.size > 1 ? 'them' : 'it' + } or remove the dependency array.` + ); + } + + let extraWarning = ''; + if (unnecessaryDependencies.size > 0) { + let badRef = null; + Array.from(unnecessaryDependencies.keys()).forEach(key => { + if (badRef !== null) { + return; + } + if (key.endsWith('.current')) { + badRef = key; + } + }); + if (badRef !== null) { + extraWarning = + ` Mutable values like '${badRef}' aren't valid dependencies ` + + "because their mutation doesn't re-render the component."; + } + } + + context.report({ + node: declaredDependenciesNode, + message: + `React Hook ${context.getSource(reactiveHook)} has ` + + // To avoid a long message, show the next actionable item. + (getWarningMessage(missingDependencies, 'a', 'missing', 'include') || + getWarningMessage( + unnecessaryDependencies, + 'an', + 'unnecessary', + 'exclude', + ) || + getWarningMessage( + duplicateDependencies, + 'a', + 'duplicate', + 'omit', + )) + + extraWarning, + fix(fixer) { + // TODO: consider preserving the comments or formatting? + return fixer.replaceText( + declaredDependenciesNode, + `[${suggestedDependencies.join(', ')}]`, + ); + }, + }); + } + }, +}; + +/** + * Assuming () means the passed/returned node: + * (props) => (props) + * props.(foo) => (props.foo) + * props.foo.(bar) => (props.foo).bar + */ +function getDependency(node) { + if ( + node.parent.type === 'MemberExpression' && + node.parent.object === node && + node.parent.property.name !== 'current' && + !node.parent.computed && + !( + node.parent.parent != null && + node.parent.parent.type === 'CallExpression' && + node.parent.parent.callee === node.parent + ) + ) { + return node.parent; + } else { + return node; + } +} + +/** + * Assuming () means the passed node. + * (foo) -> 'foo' + * foo.(bar) -> 'foo.bar' + * foo.bar.(baz) -> 'foo.bar.baz' + * Otherwise throw. + */ +function toPropertyAccessString(node) { + if (node.type === 'Identifier') { + return node.name; + } else if (node.type === 'MemberExpression' && !node.computed) { + const object = toPropertyAccessString(node.object); + const property = toPropertyAccessString(node.property); + return `${object}.${property}`; + } else { + throw new Error(`Unsupported node type: ${node.type}`); + } +} + +// What's the index of callback that needs to be analyzed for a given Hook? +// -1 if it's not a Hook we care about (e.g. useState). +// 0 for useEffect/useMemo/useCallback(fn). +// 1 for useImperativeHandle(ref, fn). +// For additionally configured Hooks, assume that they're like useEffect (0). +function getReactiveHookCallbackIndex(node, options) { + let isOnReactObject = false; + if ( + node.type === 'MemberExpression' && + node.object.type === 'Identifier' && + node.object.name === 'React' && + node.property.type === 'Identifier' && + !node.computed + ) { + node = node.property; + isOnReactObject = true; + } + if (node.type !== 'Identifier') { + return; + } + switch (node.name) { + case 'useEffect': + case 'useLayoutEffect': + case 'useCallback': + case 'useMemo': + // useEffect(fn) + return 0; + case 'useImperativeHandle': + // useImperativeHandle(ref, fn) + return 1; + default: + if (!isOnReactObject && options && options.additionalHooks) { + // Allow the user to provide a regular expression which enables the lint to + // target custom reactive hooks. + let name; + try { + name = toPropertyAccessString(node); + } catch (error) { + if (/Unsupported node type/.test(error.message)) { + return 0; + } else { + throw error; + } + } + return options.additionalHooks.test(name) ? 0 : -1; + } else { + return -1; + } + } +} + +/** + * ESLint won't assign node.parent to references from context.getScope() + * + * So instead we search for the node from an ancestor assigning node.parent + * as we go. This mutates the AST. + * + * This traversal is: + * - optimized by only searching nodes with a range surrounding our target node + * - agnostic to AST node types, it looks for `{ type: string, ... }` + */ +function fastFindReferenceWithParent(start, target) { + let queue = [start]; + let item = null; + + while (queue.length) { + item = queue.shift(); + + if (isSameIdentifier(item, target)) { + return item; + } + + if (!isAncestorNodeOf(item, target)) { + continue; + } + + for (let [key, value] of Object.entries(item)) { + if (key === 'parent') { + continue; + } + if (isNodeLike(value)) { + value.parent = item; + queue.push(value); + } else if (Array.isArray(value)) { + value.forEach(val => { + if (isNodeLike(val)) { + val.parent = item; + queue.push(val); + } + }); + } + } + } + + return null; +} + +function joinEnglish(arr) { + let s = ''; + for (let i = 0; i < arr.length; i++) { + s += arr[i]; + if (i === 0 && arr.length === 2) { + s += ' and '; + } else if (i === arr.length - 2 && arr.length > 2) { + s += ', and '; + } else if (i < arr.length - 1) { + s += ', '; + } + } + return s; +} + +function isNodeLike(val) { + return ( + typeof val === 'object' && + val !== null && + !Array.isArray(val) && + typeof val.type === 'string' + ); +} + +function isSameIdentifier(a, b) { + return ( + a.type === 'Identifier' && + a.name === b.name && + a.range[0] === b.range[0] && + a.range[1] === b.range[1] + ); +} + +function isAncestorNodeOf(a, b) { + return a.range[0] <= b.range[0] && a.range[1] >= b.range[1]; +} diff --git a/packages/eslint-plugin-react-hooks/src/index.js b/packages/eslint-plugin-react-hooks/src/index.js index a3d6216e097bc..606a6dff4e253 100644 --- a/packages/eslint-plugin-react-hooks/src/index.js +++ b/packages/eslint-plugin-react-hooks/src/index.js @@ -8,7 +8,9 @@ 'use strict'; import RuleOfHooks from './RulesOfHooks'; +import ExhaustiveDeps from './ExhaustiveDeps'; export const rules = { 'rules-of-hooks': RuleOfHooks, + 'exhaustive-deps': ExhaustiveDeps, };