From 4b6e20e0ba40b5b9d88687892f1cbf0827d773f5 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Wed, 12 Aug 2020 14:39:48 -0700 Subject: [PATCH] Handle the long tail of nodes that will be referentially unique --- .../ESLintRuleExhaustiveDeps-test.js | 170 +++++++++++++++++- .../src/ExhaustiveDeps.js | 25 +++ 2 files changed, 187 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index dc21f12d2bc23..d66a066441b6e 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1391,14 +1391,6 @@ const tests = { } `, }, - { - code: normalizeIndent` - function useFoo(){ - const foo = new MyObj(); - return useMemo(() => foo, [foo]); - } - `, - }, { code: normalizeIndent` function useFoo(){ @@ -7305,6 +7297,150 @@ const tests = { }, ], }, + { + code: normalizeIndent` + function Foo() { + const foo = <>Hi!; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' JSX fragment makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo =
Hi!
; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' JSX element makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = bar = {}; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' assignment expression makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = new String('foo'); // Note 'foo' will be boxed, and thus an object and thus compared by reference. + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object construction makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = new Map([]); + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object construction makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = /reg/; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' regular expression makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + const foo = ({}: any); + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, + { + code: normalizeIndent` + function Foo() { + class Bar {}; + useMemo(() => { + console.log(new Bar()); + }, [Bar]); + } + `, + errors: [ + { + message: + "The 'Bar' class makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'Bar' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, ], }; @@ -7683,6 +7819,24 @@ const testsTypescript = { }, ], }, + { + code: normalizeIndent` + function Foo() { + const foo = {} as any;; + useMemo(() => { + console.log(foo); + }, [foo]); + } + `, + errors: [ + { + message: + "The 'foo' object makes the dependencies of useMemo Hook (at line 6) change on every render. " + + "Move it inside the useMemo callback. Alternatively, wrap the construction of 'foo' in its own useMemo() Hook.", + suggestions: undefined, + }, + ], + }, ], }; diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index f157b3aea5c6b..5cb1974721715 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -1428,6 +1428,26 @@ function getConstructionExpresionType(node) { return 'logical expression'; } return null; + case 'JSXFragment': + return 'JSX fragment'; + case 'JSXElement': + return 'JSX element'; + case 'AssignmentExpression': + if (getConstructionExpresionType(node.right) != null) { + return 'assignment expression'; + } + return null; + case 'NewExpression': + return 'object construction'; + case 'Literal': + if (node.value instanceof RegExp) { + return 'regular expression'; + } + return null; + case 'TypeCastExpression': + return getConstructionExpresionType(node.expression); + case 'TSAsExpression': + return getConstructionExpresionType(node.expression); } return null; } @@ -1482,6 +1502,11 @@ function scanForConstructions({ ) { return [ref, 'function']; } + + // class Foo {} + if (node.type === 'ClassName' && node.node.type === 'ClassDeclaration') { + return [ref, 'class']; + } return null; }) .filter(Boolean);