diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
index de6d0aa4f7e07..26cda5e290875 100644
--- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
@@ -56,7 +56,7 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
useEffect(() => {
console.log(local);
}, [local]);
@@ -94,9 +94,9 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local1 = {};
+ const local1 = someFunc();
{
- const local2 = {};
+ const local2 = someFunc();
useCallback(() => {
console.log(local1);
console.log(local2);
@@ -108,9 +108,9 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local1 = {};
+ const local1 = someFunc();
function MyNestedComponent() {
- const local2 = {};
+ const local2 = someFunc();
useCallback(() => {
console.log(local1);
console.log(local2);
@@ -122,7 +122,7 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
useEffect(() => {
console.log(local);
console.log(local);
@@ -142,7 +142,7 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
useEffect(() => {
console.log(local);
}, [,,,local,,,]);
@@ -222,7 +222,7 @@ const tests = {
{
code: normalizeIndent`
function MyComponent(props) {
- const local = {};
+ const local = someFunc();
useEffect(() => {
console.log(props.foo);
console.log(props.bar);
@@ -243,7 +243,7 @@ const tests = {
console.log(props.bar);
}, [props, props.foo]);
- let color = {}
+ let color = someFunc();
useEffect(() => {
console.log(props.foo.bar.baz);
console.log(color);
@@ -416,7 +416,7 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
function myEffect() {
console.log(local);
}
@@ -731,7 +731,7 @@ const tests = {
// direct assignments.
code: normalizeIndent`
function MyComponent(props) {
- let obj = {};
+ let obj = someFunc();
useEffect(() => {
obj.foo = true;
}, [obj]);
@@ -1376,6 +1376,64 @@ const tests = {
}
`,
},
+ {
+ code: normalizeIndent`
+ function useFoo(foo){
+ return useMemo(() => foo, [foo]);
+ }
+ `,
+ },
+ {
+ code: normalizeIndent`
+ function useFoo(){
+ const foo = "hi!";
+ return useMemo(() => foo, [foo]);
+ }
+ `,
+ },
+ {
+ code: normalizeIndent`
+ function useFoo(){
+ let {foo} = {foo: 1};
+ return useMemo(() => foo, [foo]);
+ }
+ `,
+ },
+ {
+ code: normalizeIndent`
+ function useFoo(){
+ let [foo] = [1];
+ return useMemo(() => foo, [foo]);
+ }
+ `,
+ },
+ {
+ code: normalizeIndent`
+ function useFoo() {
+ const foo = "fine";
+ if (true) {
+ // Shadowed variable with constant construction in a nested scope is fine.
+ const foo = {};
+ }
+ return useMemo(() => foo, [foo]);
+ }
+ `,
+ },
+ {
+ code: normalizeIndent`
+ function MyComponent({foo}) {
+ return useMemo(() => foo, [foo])
+ }
+ `,
+ },
+ {
+ code: normalizeIndent`
+ function MyComponent() {
+ const foo = true ? "fine" : "also fine";
+ return useMemo(() => foo, [foo]);
+ }
+ `,
+ },
],
invalid: [
{
@@ -1494,7 +1552,7 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
useEffect(() => {
console.log(local);
}, []);
@@ -1510,7 +1568,7 @@ const tests = {
desc: 'Update the dependencies array to be: [local]',
output: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
useEffect(() => {
console.log(local);
}, [local]);
@@ -1636,7 +1694,7 @@ const tests = {
// Regression test
code: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
useEffect(() => {
if (true) {
console.log(local);
@@ -1654,7 +1712,7 @@ const tests = {
desc: 'Update the dependencies array to be: [local]',
output: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
useEffect(() => {
if (true) {
console.log(local);
@@ -1742,9 +1800,9 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local1 = {};
+ const local1 = someFunc();
{
- const local2 = {};
+ const local2 = someFunc();
useEffect(() => {
console.log(local1);
console.log(local2);
@@ -1762,9 +1820,9 @@ const tests = {
desc: 'Update the dependencies array to be: [local1, local2]',
output: normalizeIndent`
function MyComponent() {
- const local1 = {};
+ const local1 = someFunc();
{
- const local2 = {};
+ const local2 = someFunc();
useEffect(() => {
console.log(local1);
console.log(local2);
@@ -1846,7 +1904,7 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local1 = {};
+ const local1 = someFunc();
function MyNestedComponent() {
const local2 = {};
useCallback(() => {
@@ -1868,7 +1926,7 @@ const tests = {
desc: 'Update the dependencies array to be: [local2]',
output: normalizeIndent`
function MyComponent() {
- const local1 = {};
+ const local1 = someFunc();
function MyNestedComponent() {
const local2 = {};
useCallback(() => {
@@ -2295,7 +2353,7 @@ const tests = {
{
code: normalizeIndent`
function MyComponent() {
- const local = {};
+ const local = someFunc();
useEffect(() => {
console.log(local);
}, [local, ...dependencies]);
@@ -5311,7 +5369,7 @@ const tests = {
`The 'handleNext' function makes the dependencies of ` +
`useEffect Hook (at line 11) change on every render. ` +
`Move it inside the useEffect callback. Alternatively, ` +
- `wrap the 'handleNext' definition into its own useCallback() Hook.`,
+ `wrap the definition of 'handleNext' in its own useCallback() Hook.`,
// Not gonna fix a function definition
// because it's not always safe due to hoisting.
suggestions: undefined,
@@ -5340,7 +5398,7 @@ const tests = {
`The 'handleNext' function makes the dependencies of ` +
`useEffect Hook (at line 11) change on every render. ` +
`Move it inside the useEffect callback. Alternatively, ` +
- `wrap the 'handleNext' definition into its own useCallback() Hook.`,
+ `wrap the definition of 'handleNext' in its own useCallback() Hook.`,
// We don't fix moving (too invasive). But that's the suggested fix
// when only effect uses this function. Otherwise, we'd useCallback.
suggestions: undefined,
@@ -5373,13 +5431,13 @@ const tests = {
message:
`The 'handleNext' function makes the dependencies of ` +
`useEffect Hook (at line 11) change on every render. ` +
- `To fix this, wrap the 'handleNext' definition into its own useCallback() Hook.`,
+ `To fix this, wrap the definition of 'handleNext' in its own useCallback() Hook.`,
// We fix this one with useCallback since it's
// the easy fix and you can't just move it into effect.
suggestions: [
{
desc:
- "Wrap the 'handleNext' definition into its own useCallback() Hook.",
+ "Wrap the definition of 'handleNext' in its own useCallback() Hook.",
output: normalizeIndent`
function MyComponent(props) {
let [, setState] = useState();
@@ -5428,21 +5486,21 @@ const tests = {
message:
"The 'handleNext1' function makes the dependencies of useEffect Hook " +
'(at line 14) change on every render. Move it inside the useEffect callback. ' +
- "Alternatively, wrap the 'handleNext1' definition into its own useCallback() Hook.",
+ "Alternatively, wrap the definition of 'handleNext1' in its own useCallback() Hook.",
suggestions: undefined,
},
{
message:
"The 'handleNext2' function makes the dependencies of useLayoutEffect Hook " +
'(at line 17) change on every render. Move it inside the useLayoutEffect callback. ' +
- "Alternatively, wrap the 'handleNext2' definition into its own useCallback() Hook.",
+ "Alternatively, wrap the definition of 'handleNext2' in its own useCallback() Hook.",
suggestions: undefined,
},
{
message:
"The 'handleNext3' function makes the dependencies of useMemo Hook " +
'(at line 20) change on every render. Move it inside the useMemo callback. ' +
- "Alternatively, wrap the 'handleNext3' definition into its own useCallback() Hook.",
+ "Alternatively, wrap the definition of 'handleNext3' in its own useCallback() Hook.",
suggestions: undefined,
},
],
@@ -5480,21 +5538,21 @@ const tests = {
message:
"The 'handleNext1' function makes the dependencies of useEffect Hook " +
'(at line 15) change on every render. Move it inside the useEffect callback. ' +
- "Alternatively, wrap the 'handleNext1' definition into its own useCallback() Hook.",
+ "Alternatively, wrap the definition of 'handleNext1' in its own useCallback() Hook.",
suggestions: undefined,
},
{
message:
"The 'handleNext2' function makes the dependencies of useLayoutEffect Hook " +
'(at line 19) change on every render. Move it inside the useLayoutEffect callback. ' +
- "Alternatively, wrap the 'handleNext2' definition into its own useCallback() Hook.",
+ "Alternatively, wrap the definition of 'handleNext2' in its own useCallback() Hook.",
suggestions: undefined,
},
{
message:
"The 'handleNext3' function makes the dependencies of useMemo Hook " +
'(at line 23) change on every render. Move it inside the useMemo callback. ' +
- "Alternatively, wrap the 'handleNext3' definition into its own useCallback() Hook.",
+ "Alternatively, wrap the definition of 'handleNext3' in its own useCallback() Hook.",
suggestions: undefined,
},
],
@@ -5541,20 +5599,20 @@ const tests = {
message:
"The 'handleNext1' function makes the dependencies of useEffect Hook " +
'(at line 15) change on every render. To fix this, wrap the ' +
- "'handleNext1' definition into its own useCallback() Hook.",
+ "definition of 'handleNext1' in its own useCallback() Hook.",
suggestions: undefined,
},
{
message:
"The 'handleNext2' function makes the dependencies of useLayoutEffect Hook " +
'(at line 19) change on every render. To fix this, wrap the ' +
- "'handleNext2' definition into its own useCallback() Hook.",
+ "definition of 'handleNext2' in its own useCallback() Hook.",
// Suggestion wraps into useCallback where possible (variables only)
// because they are only referenced outside the effect.
suggestions: [
{
desc:
- "Wrap the 'handleNext2' definition into its own useCallback() Hook.",
+ "Wrap the definition of 'handleNext2' in its own useCallback() Hook.",
output: normalizeIndent`
function MyComponent(props) {
function handleNext1() {
@@ -5598,13 +5656,13 @@ const tests = {
message:
"The 'handleNext3' function makes the dependencies of useMemo Hook " +
'(at line 23) change on every render. To fix this, wrap the ' +
- "'handleNext3' definition into its own useCallback() Hook.",
+ "definition of 'handleNext3' in its own useCallback() Hook.",
// Autofix wraps into useCallback where possible (variables only)
// because they are only referenced outside the effect.
suggestions: [
{
desc:
- "Wrap the 'handleNext3' definition into its own useCallback() Hook.",
+ "Wrap the definition of 'handleNext3' in its own useCallback() Hook.",
output: normalizeIndent`
function MyComponent(props) {
function handleNext1() {
@@ -5675,11 +5733,11 @@ const tests = {
message:
"The 'handleNext1' function makes the dependencies of useEffect Hook " +
'(at line 12) change on every render. To fix this, wrap the ' +
- "'handleNext1' definition into its own useCallback() Hook.",
+ "definition of 'handleNext1' in its own useCallback() Hook.",
suggestions: [
{
desc:
- "Wrap the 'handleNext1' definition into its own useCallback() Hook.",
+ "Wrap the definition of 'handleNext1' in its own useCallback() Hook.",
output: normalizeIndent`
function MyComponent(props) {
const handleNext1 = useCallback(() => {
@@ -5705,11 +5763,11 @@ const tests = {
message:
"The 'handleNext1' function makes the dependencies of useEffect Hook " +
'(at line 16) change on every render. To fix this, wrap the ' +
- "'handleNext1' definition into its own useCallback() Hook.",
+ "definition of 'handleNext1' in its own useCallback() Hook.",
suggestions: [
{
desc:
- "Wrap the 'handleNext1' definition into its own useCallback() Hook.",
+ "Wrap the definition of 'handleNext1' in its own useCallback() Hook.",
output: normalizeIndent`
function MyComponent(props) {
const handleNext1 = useCallback(() => {
@@ -5735,14 +5793,14 @@ const tests = {
message:
"The 'handleNext2' function makes the dependencies of useEffect Hook " +
'(at line 12) change on every render. To fix this, wrap the ' +
- "'handleNext2' definition into its own useCallback() Hook.",
+ "definition of 'handleNext2' in its own useCallback() Hook.",
suggestions: undefined,
},
{
message:
"The 'handleNext2' function makes the dependencies of useEffect Hook " +
'(at line 16) change on every render. To fix this, wrap the ' +
- "'handleNext2' definition into its own useCallback() Hook.",
+ "definition of 'handleNext2' in its own useCallback() Hook.",
suggestions: undefined,
},
],
@@ -5767,8 +5825,8 @@ const tests = {
{
message:
"The 'handleNext' function makes the dependencies of useEffect Hook " +
- '(at line 13) change on every render. To fix this, wrap the ' +
- "'handleNext' definition into its own useCallback() Hook.",
+ '(at line 13) change on every render. To fix this, wrap the definition of ' +
+ "'handleNext' in its own useCallback() Hook.",
// Normally we'd suggest moving handleNext inside an
// effect. But it's used more than once.
// TODO: our autofix here isn't quite sufficient because
@@ -5776,7 +5834,7 @@ const tests = {
suggestions: [
{
desc:
- "Wrap the 'handleNext' definition into its own useCallback() Hook.",
+ "Wrap the definition of 'handleNext' in its own useCallback() Hook.",
output: normalizeIndent`
function MyComponent(props) {
let handleNext = useCallback(() => {
@@ -5820,7 +5878,7 @@ const tests = {
`The 'handleNext' function makes the dependencies of ` +
`useEffect Hook (at line 14) change on every render. ` +
`Move it inside the useEffect callback. Alternatively, wrap the ` +
- `'handleNext' definition into its own useCallback() Hook.`,
+ `definition of 'handleNext' in its own useCallback() Hook.`,
suggestions: undefined,
},
],
@@ -6085,7 +6143,7 @@ const tests = {
message:
`The 'increment' function makes the dependencies of useEffect Hook ` +
`(at line 14) change on every render. Move it inside the useEffect callback. ` +
- `Alternatively, wrap the \'increment\' definition into its own ` +
+ `Alternatively, wrap the definition of \'increment\' in its own ` +
`useCallback() Hook.`,
suggestions: undefined,
},
@@ -6967,6 +7025,499 @@ const tests = {
},
],
},
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = {};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = [];
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' array makes the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = () => {};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' function makes the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the definition of 'foo' in its own " +
+ 'useCallback() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = function bar(){};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' function makes the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the definition of 'foo' in its own " +
+ 'useCallback() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = class {};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' class makes the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = true ? {} : "fine";
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' conditional could make the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = bar || {};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' logical expression could make the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = bar ?? {};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' logical expression could make the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = bar && {};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' logical expression could make the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = bar ? baz ? {} : null : null;
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' conditional could make the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ let foo = {};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ var foo = {};
+ useMemo(() => foo, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useMemo Hook (at line 4) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = {};
+ useCallback(() => {
+ console.log(foo);
+ }, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useCallback Hook (at line 6) change on every render. " +
+ "Move it inside the useCallback callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = {};
+ useEffect(() => {
+ console.log(foo);
+ }, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useEffect Hook (at line 6) change on every render. " +
+ "Move it inside the useEffect callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = {};
+ useLayoutEffect(() => {
+ console.log(foo);
+ }, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useLayoutEffect Hook (at line 6) change on every render. " +
+ "Move it inside the useLayoutEffect callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Component() {
+ const foo = {};
+ useImperativeHandle(
+ ref,
+ () => {
+ console.log(foo);
+ },
+ [foo]
+ );
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useImperativeHandle Hook (at line 9) change on every render. " +
+ "Move it inside the useImperativeHandle callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Foo(section) {
+ const foo = section.section_components?.edges ?? [];
+ useEffect(() => {
+ console.log(foo);
+ }, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' logical expression could make the dependencies of useEffect Hook (at line 6) change on every render. " +
+ "Move it inside the useEffect callback. Alternatively, wrap the initialization of 'foo' in its own " +
+ 'useMemo() Hook.',
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Foo(section) {
+ const foo = {};
+ console.log(foo);
+ useMemo(() => {
+ console.log(foo);
+ }, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useMemo Hook (at line 7) change on every render. " +
+ "To fix this, wrap the initialization 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 fragment makes the dependencies of useMemo Hook (at line 6) change on every render. " +
+ "Move it inside the useMemo callback. Alternatively, wrap the initialization of 'foo' in its own useMemo() Hook.",
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Foo() {
+ const foo = <div>Hi!</div>;
+ 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 initialization 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 initialization 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 initialization 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 initialization 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 initialization 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 initialization 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 initialization of 'Bar' in its own useMemo() Hook.",
+ suggestions: undefined,
+ },
+ ],
+ },
+ {
+ code: normalizeIndent`
+ function Foo() {
+ const foo = {};
+ useLayoutEffect(() => {
+ console.log(foo);
+ }, [foo]);
+ useEffect(() => {
+ console.log(foo);
+ }, [foo]);
+ }
+ `,
+ errors: [
+ {
+ message:
+ "The 'foo' object makes the dependencies of useLayoutEffect Hook (at line 6) change on every render. " +
+ "To fix this, wrap the initialization of 'foo' in its own useMemo() Hook.",
+ suggestions: undefined,
+ },
+ {
+ message:
+ "The 'foo' object makes the dependencies of useEffect Hook (at line 9) change on every render. " +
+ "To fix this, wrap the initialization of 'foo' in its own useMemo() Hook.",
+ suggestions: undefined,
+ },
+ ],
+ },
],
};
@@ -7345,6 +7896,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 initialization 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 9e344e8662d6d..ab552c305a4db 100644
--- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
@@ -844,59 +844,80 @@ export default {
unnecessaryDependencies.size;
if (problemCount === 0) {
- // If nothing else to report, check if some callbacks
- // are bare and would invalidate on every render.
- const bareFunctions = scanForDeclaredBareFunctions({
+ // If nothing else to report, check if some dependencies would
+ // invalidate on every render.
+ const constructions = scanForConstructions({
declaredDependencies,
declaredDependenciesNode,
componentScope,
scope,
});
- bareFunctions.forEach(({fn, suggestUseCallback}) => {
- let message =
- `The '${fn.name.name}' function makes the dependencies of ` +
- `${reactiveHookName} Hook (at line ${declaredDependenciesNode.loc.start.line}) ` +
- `change on every render.`;
- if (suggestUseCallback) {
- message +=
- ` To fix this, ` +
- `wrap the '${fn.name.name}' definition into its own useCallback() Hook.`;
- } else {
- message +=
- ` Move it inside the ${reactiveHookName} callback. ` +
- `Alternatively, wrap the '${fn.name.name}' definition into its own useCallback() Hook.`;
- }
+ constructions.forEach(
+ ({construction, isUsedOutsideOfHook, depType}) => {
+ const wrapperHook =
+ depType === 'function' ? 'useCallback' : 'useMemo';
- let suggest;
- // Only handle the simple case: arrow functions.
- // Wrapping function declarations can mess up hoisting.
- if (suggestUseCallback && fn.type === 'Variable') {
- suggest = [
- {
- desc: `Wrap the '${fn.name.name}' definition into its own useCallback() Hook.`,
- fix(fixer) {
- return [
- // TODO: also add an import?
- fixer.insertTextBefore(fn.node.init, 'useCallback('),
- // TODO: ideally we'd gather deps here but it would require
- // restructuring the rule code. This will cause a new lint
- // error to appear immediately for useCallback. Note we're
- // not adding [] because would that changes semantics.
- fixer.insertTextAfter(fn.node.init, ')'),
- ];
+ const constructionType =
+ depType === 'function' ? 'definition' : 'initialization';
+
+ const defaultAdvice = `wrap the ${constructionType} of '${construction.name.name}' in its own ${wrapperHook}() Hook.`;
+
+ const advice = isUsedOutsideOfHook
+ ? `To fix this, ${defaultAdvice}`
+ : `Move it inside the ${reactiveHookName} callback. Alternatively, ${defaultAdvice}`;
+
+ const causation =
+ depType === 'conditional' || depType === 'logical expression'
+ ? 'could make'
+ : 'makes';
+
+ const message =
+ `The '${construction.name.name}' ${depType} ${causation} the dependencies of ` +
+ `${reactiveHookName} Hook (at line ${declaredDependenciesNode.loc.start.line}) ` +
+ `change on every render. ${advice}`;
+
+ let suggest;
+ // Only handle the simple case of variable assignments.
+ // Wrapping function declarations can mess up hoisting.
+ if (
+ isUsedOutsideOfHook &&
+ construction.type === 'Variable' &&
+ // Objects may be mutated ater construction, which would make this
+ // fix unsafe. Functions _probably_ won't be mutated, so we'll
+ // allow this fix for them.
+ depType === 'function'
+ ) {
+ suggest = [
+ {
+ desc: `Wrap the ${constructionType} of '${construction.name.name}' in its own ${wrapperHook}() Hook.`,
+ fix(fixer) {
+ const [before, after] =
+ wrapperHook === 'useMemo'
+ ? [`useMemo(() => { return `, '; })']
+ : ['useCallback(', ')'];
+ return [
+ // TODO: also add an import?
+ fixer.insertTextBefore(construction.node.init, before),
+ // TODO: ideally we'd gather deps here but it would require
+ // restructuring the rule code. This will cause a new lint
+ // error to appear immediately for useCallback. Note we're
+ // not adding [] because would that changes semantics.
+ fixer.insertTextAfter(construction.node.init, after),
+ ];
+ },
},
- },
- ];
- }
- // TODO: What if the function needs to change on every render anyway?
- // Should we suggest removing effect deps as an appropriate fix too?
- reportProblem({
- // TODO: Why not report this at the dependency site?
- node: fn.node,
- message,
- suggest,
- });
- });
+ ];
+ }
+ // TODO: What if the function needs to change on every render anyway?
+ // Should we suggest removing effect deps as an appropriate fix too?
+ reportProblem({
+ // TODO: Why not report this at the dependency site?
+ node: construction.node,
+ message,
+ suggest,
+ });
+ },
+ );
return;
}
@@ -1381,50 +1402,116 @@ function collectRecommendations({
};
}
-// Finds functions declared as dependencies
+// If the node will result in constructing a referentially unique value, return
+// its human readable type name, else return null.
+function getConstructionExpressionType(node) {
+ switch (node.type) {
+ case 'ObjectExpression':
+ return 'object';
+ case 'ArrayExpression':
+ return 'array';
+ case 'ArrowFunctionExpression':
+ case 'FunctionExpression':
+ return 'function';
+ case 'ClassExpression':
+ return 'class';
+ case 'ConditionalExpression':
+ if (
+ getConstructionExpressionType(node.consequent) != null ||
+ getConstructionExpressionType(node.alternate) != null
+ ) {
+ return 'conditional';
+ }
+ return null;
+ case 'LogicalExpression':
+ if (
+ getConstructionExpressionType(node.left) != null ||
+ getConstructionExpressionType(node.right) != null
+ ) {
+ return 'logical expression';
+ }
+ return null;
+ case 'JSXFragment':
+ return 'JSX fragment';
+ case 'JSXElement':
+ return 'JSX element';
+ case 'AssignmentExpression':
+ if (getConstructionExpressionType(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 getConstructionExpressionType(node.expression);
+ case 'TSAsExpression':
+ return getConstructionExpressionType(node.expression);
+ }
+ return null;
+}
+
+// Finds variables declared as dependencies
// that would invalidate on every render.
-function scanForDeclaredBareFunctions({
+function scanForConstructions({
declaredDependencies,
declaredDependenciesNode,
componentScope,
scope,
}) {
- const bareFunctions = declaredDependencies
+ const constructions = declaredDependencies
.map(({key}) => {
- const fnRef = componentScope.set.get(key);
- if (fnRef == null) {
+ const ref = componentScope.variables.find(v => v.name === key);
+ if (ref == null) {
return null;
}
- const fnNode = fnRef.defs[0];
- if (fnNode == null) {
+
+ const node = ref.defs[0];
+ if (node == null) {
return null;
}
// const handleChange = function () {}
// const handleChange = () => {}
+ // const foo = {}
+ // const foo = []
+ // etc.
if (
- fnNode.type === 'Variable' &&
- fnNode.node.type === 'VariableDeclarator' &&
- fnNode.node.init != null &&
- (fnNode.node.init.type === 'ArrowFunctionExpression' ||
- fnNode.node.init.type === 'FunctionExpression')
+ node.type === 'Variable' &&
+ node.node.type === 'VariableDeclarator' &&
+ node.node.id.type === 'Identifier' && // Ensure this is not destructed assignment
+ node.node.init != null
) {
- return fnRef;
+ const constantExpressionType = getConstructionExpressionType(
+ node.node.init,
+ );
+ if (constantExpressionType != null) {
+ return [ref, constantExpressionType];
+ }
}
// function handleChange() {}
if (
- fnNode.type === 'FunctionName' &&
- fnNode.node.type === 'FunctionDeclaration'
+ node.type === 'FunctionName' &&
+ node.node.type === 'FunctionDeclaration'
) {
- return fnRef;
+ return [ref, 'function'];
+ }
+
+ // class Foo {}
+ if (node.type === 'ClassName' && node.node.type === 'ClassDeclaration') {
+ return [ref, 'class'];
}
return null;
})
.filter(Boolean);
- function isUsedOutsideOfHook(fnRef) {
+ function isUsedOutsideOfHook(ref) {
let foundWriteExpr = false;
- for (let i = 0; i < fnRef.references.length; i++) {
- const reference = fnRef.references[i];
+ for (let i = 0; i < ref.references.length; i++) {
+ const reference = ref.references[i];
if (reference.writeExpr) {
if (foundWriteExpr) {
// Two writes to the same function.
@@ -1450,9 +1537,10 @@ function scanForDeclaredBareFunctions({
return false;
}
- return bareFunctions.map(fnRef => ({
- fn: fnRef.defs[0],
- suggestUseCallback: isUsedOutsideOfHook(fnRef),
+ return constructions.map(([ref, depType]) => ({
+ construction: ref.defs[0],
+ depType,
+ isUsedOutsideOfHook: isUsedOutsideOfHook(ref),
}));
}