@samitouri / QOS-React / commits / 1204c78977

[eslint] Wording tweaks (#15078)

* [eslint] Wording tweaks I think these are a little clearer. * fix tests

Sophie Alpert committed Mar 13, 2019 at 11:31 UTC 1204c789776cb01fbaf3e9f032e7e2ba85a44137

3 files changed +170 -156

fixtures/eslint/.eslintrc.json

+1 -1

@@ -1,7 +1,7 @@

1 {

2 "root": true,

3 "parserOptions": {

4 - "ecmaVersion": 6,

4 + "ecmaVersion": 8,

5 "sourceType": "module",

6 "ecmaFeatures": {

7 "jsx": true

packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js

+122 -112

@@ -1060,12 +1060,10 @@ const tests = {

1060 }

1061 `,

1062 errors: [

1063 - "React Hook useMemo doesn't serve any purpose without a dependency array. " +

1064 - 'To enable this optimization, pass an array of values used by the inner ' +

1065 - 'function as the second argument to useMemo.',

1066 - "React Hook useCallback doesn't serve any purpose without a dependency array. " +

1067 - 'To enable this optimization, pass an array of values used by the inner ' +

1068 - 'function as the second argument to useCallback.',

1063 + 'React Hook useMemo does nothing when called with only one argument. ' +

1064 + 'Did you forget to pass an array of dependencies?',

1065 + 'React Hook useCallback does nothing when called with only one argument. ' +

1066 + 'Did you forget to pass an array of dependencies?',

1067 ],

1068 },

1069 {

@@ -1260,7 +1258,7 @@ const tests = {

1258 "React Hook useCallback has a missing dependency: 'local2'. " +

1259 'Either include it or remove the dependency array. ' +

1260 "Outer scope values like 'local1' aren't valid dependencies " +

1263 - "because their mutation doesn't re-render the component.",

1261 + "because mutating them doesn't re-render the component.",

1262 ],

1263 },

1264 {

@@ -1326,7 +1324,7 @@ const tests = {

1324 "React Hook useCallback has an unnecessary dependency: 'window'. " +

1325 'Either exclude it or remove the dependency array. ' +

1326 "Outer scope values like 'window' aren't valid dependencies " +

1329 - "because their mutation doesn't re-render the component.",

1327 + "because mutating them doesn't re-render the component.",

1328 ],

1329 },

1330 {

@@ -1395,6 +1393,24 @@ const tests = {

1393 'Either include it or remove the dependency array.',

1394 ],

1395 },

1396 + {

1397 + code: `

1398 + function MyComponent() {

1399 + useEffect(() => {}, ['foo']);

1400 + }

1401 + `,

1402 + // TODO: we could autofix this.

1403 + output: `

1404 + function MyComponent() {

1405 + useEffect(() => {}, ['foo']);

1406 + }

1407 + `,

1408 + errors: [

1409 + // Don't assume user meant `foo` because it's not used in the effect.

1410 + "The 'foo' literal is not a valid dependency because it never changes. " +

1411 + 'You can safely remove it.',

1412 + ],

1413 + },

1414 {

1415 code: `

1416 function MyComponent({ foo, bar, baz }) {

@@ -1413,9 +1429,9 @@ const tests = {

1429 errors: [

1430 "React Hook useEffect has missing dependencies: 'bar', 'baz', and 'foo'. " +

1431 'Either include them or remove the dependency array.',

1416 - "The 'foo' string literal is not a valid dependency because it never changes. " +

1432 + "The 'foo' literal is not a valid dependency because it never changes. " +

1433 'Did you mean to include foo in the array instead?',

1418 - "The 'bar' string literal is not a valid dependency because it never changes. " +

1434 + "The 'bar' literal is not a valid dependency because it never changes. " +

1435 'Did you mean to include bar in the array instead?',

1436 ],

1437 },

@@ -1437,9 +1453,9 @@ const tests = {

1453 errors: [

1454 "React Hook useEffect has missing dependencies: 'bar', 'baz', and 'foo'. " +

1455 'Either include them or remove the dependency array.',

1440 - "The '42' literal is not a valid dependency because it never changes. You can safely remove it.",

1441 - "The 'false' literal is not a valid dependency because it never changes. You can safely remove it.",

1442 - "The 'null' literal is not a valid dependency because it never changes. You can safely remove it.",

1456 + 'The 42 literal is not a valid dependency because it never changes. You can safely remove it.',

1457 + 'The false literal is not a valid dependency because it never changes. You can safely remove it.',

1458 + 'The null literal is not a valid dependency because it never changes. You can safely remove it.',

1459 ],

1460 },

1461 {

@@ -1456,8 +1472,8 @@ const tests = {

1472 }

1473 `,

1474 errors: [

1459 - 'React Hook useEffect has a second argument which is not an array ' +

1460 - "literal. This means we can't statically verify whether you've " +

1475 + 'React Hook useEffect was passed a dependency list that is not an ' +

1476 + "array literal. This means we can't statically verify whether you've " +

1477 'passed the correct dependencies.',

1478 ],

1479 },

@@ -1482,8 +1498,8 @@ const tests = {

1498 }

1499 `,

1500 errors: [

1485 - 'React Hook useEffect has a second argument which is not an array ' +

1486 - "literal. This means we can't statically verify whether you've " +

1501 + 'React Hook useEffect was passed a dependency list that is not an ' +

1502 + "array literal. This means we can't statically verify whether you've " +

1503 'passed the correct dependencies.',

1504 "React Hook useEffect has a missing dependency: 'local'. " +

1505 'Either include it or remove the dependency array.',

@@ -2327,8 +2343,8 @@ const tests = {

2343 errors: [

2344 "React Hook useEffect has a missing dependency: 'state'. " +

2345 'Either include it or remove the dependency array. ' +

2330 - `You can also write 'setState(s => ...)' ` +

2331 - `if you only use 'state' for the 'setState' call.`,

2346 + `You can also do a functional update 'setState(s => ...)' ` +

2347 + `if you only need 'state' in the 'setState' call.`,

2348 ],

2349 },

2350 {

@@ -2358,8 +2374,8 @@ const tests = {

2374 errors: [

2375 "React Hook useEffect has a missing dependency: 'state'. " +

2376 'Either include it or remove the dependency array. ' +

2361 - `You can also write 'setState(s => ...)' ` +

2362 - `if you only use 'state' for the 'setState' call.`,

2377 + `You can also do a functional update 'setState(s => ...)' ` +

2378 + `if you only need 'state' in the 'setState' call.`,

2379 ],

2380 },

2381 {

@@ -2421,7 +2437,7 @@ const tests = {

2437 "React Hook useEffect has unnecessary dependencies: 'ref1.current' and 'ref2.current'. " +

2438 'Either exclude them or remove the dependency array. ' +

2439 "Mutable values like 'ref1.current' aren't valid dependencies " +

2424 - "because their mutation doesn't re-render the component.",

2440 + "because mutating them doesn't re-render the component.",

2441 ],

2442 },

2443 {

@@ -2445,7 +2461,7 @@ const tests = {

2461 "React Hook useEffect has an unnecessary dependency: 'ref.current'. " +

2462 'Either exclude it or remove the dependency array. ' +

2463 "Mutable values like 'ref.current' aren't valid dependencies " +

2448 - "because their mutation doesn't re-render the component.",

2464 + "because mutating them doesn't re-render the component.",

2465 ],

2466 },

2467 {

@@ -2473,7 +2489,7 @@ const tests = {

2489 "React Hook useEffect has unnecessary dependencies: 'ref1.current' and 'ref2.current'. " +

2490 'Either exclude them or remove the dependency array. ' +

2491 "Mutable values like 'ref1.current' aren't valid dependencies " +

2476 - "because their mutation doesn't re-render the component.",

2492 + "because mutating them doesn't re-render the component.",

2493 ],

2494 },

2495 {

@@ -2501,7 +2517,7 @@ const tests = {

2517 "React Hook useCallback has unnecessary dependencies: 'activeTab', 'ref1.current', and 'ref2.current'. " +

2518 'Either exclude them or remove the dependency array. ' +

2519 "Mutable values like 'ref1.current' aren't valid dependencies " +

2504 - "because their mutation doesn't re-render the component.",

2520 + "because mutating them doesn't re-render the component.",

2521 ],

2522 },

2523 {

@@ -2525,7 +2541,7 @@ const tests = {

2541 "React Hook useEffect has an unnecessary dependency: 'ref.current'. " +

2542 'Either exclude it or remove the dependency array. ' +

2543 "Mutable values like 'ref.current' aren't valid dependencies " +

2528 - "because their mutation doesn't re-render the component.",

2544 + "because mutating them doesn't re-render the component.",

2545 ],

2546 },

2547 {

@@ -2574,9 +2590,10 @@ const tests = {

2590 errors: [

2591 "React Hook useEffect has a missing dependency: 'props'. " +

2592 'Either include it or remove the dependency array. ' +

2577 - `However, the preferred fix is to destructure the 'props' ` +

2578 - `object outside of the useEffect call and refer to specific ` +

2579 - `props directly by their names.`,

2593 + `However, 'props' will change when *any* prop changes, so the ` +

2594 + `preferred fix is to destructure the 'props' object outside ` +

2595 + `of the useEffect call and refer to those specific ` +

2596 + `props inside useEffect.`,

2597 ],

2598 },

2599 {

@@ -2607,9 +2624,10 @@ const tests = {

2624 errors: [

2625 "React Hook useEffect has a missing dependency: 'props'. " +

2626 'Either include it or remove the dependency array. ' +

2610 - `However, the preferred fix is to destructure the 'props' ` +

2611 - `object outside of the useEffect call and refer to specific ` +

2612 - `props directly by their names.`,

2627 + `However, 'props' will change when *any* prop changes, so the ` +

2628 + `preferred fix is to destructure the 'props' object outside ` +

2629 + `of the useEffect call and refer to those specific ` +

2630 + `props inside useEffect.`,

2631 ],

2632 },

2633 {

@@ -2660,9 +2678,10 @@ const tests = {

2678 errors: [

2679 "React Hook useEffect has a missing dependency: 'props'. " +

2680 'Either include it or remove the dependency array. ' +

2663 - `However, the preferred fix is to destructure the 'props' ` +

2664 - `object outside of the useEffect call and refer to specific ` +

2665 - `props directly by their names.`,

2681 + `However, 'props' will change when *any* prop changes, so the ` +

2682 + `preferred fix is to destructure the 'props' object outside ` +

2683 + `of the useEffect call and refer to those specific ` +

2684 + `props inside useEffect.`,

2685 ],

2686 },

2687 {

@@ -2689,9 +2708,10 @@ const tests = {

2708 errors: [

2709 "React Hook useEffect has a missing dependency: 'props'. " +

2710 'Either include it or remove the dependency array. ' +

2692 - `However, the preferred fix is to destructure the 'props' ` +

2693 - `object outside of the useEffect call and refer to specific ` +

2694 - `props directly by their names.`,

2711 + `However, 'props' will change when *any* prop changes, so the ` +

2712 + `preferred fix is to destructure the 'props' object outside ` +

2713 + `of the useEffect call and refer to those specific ` +

2714 + `props inside useEffect.`,

2715 ],

2716 },

2717 {

@@ -2718,9 +2738,10 @@ const tests = {

2738 errors: [

2739 "React Hook useEffect has missing dependencies: 'props' and 'skillsCount'. " +

2740 'Either include them or remove the dependency array. ' +

2721 - `However, the preferred fix is to destructure the 'props' ` +

2722 - `object outside of the useEffect call and refer to specific ` +

2723 - `props directly by their names.`,

2741 + `However, 'props' will change when *any* prop changes, so the ` +

2742 + `preferred fix is to destructure the 'props' object outside ` +

2743 + `of the useEffect call and refer to those specific ` +

2744 + `props inside useEffect.`,

2745 ],

2746 },

2747 {

@@ -2819,29 +2840,25 @@ const tests = {

2840 `,

2841 errors: [

2842 // value2

2822 - `Assignments to the 'value2' variable from inside a React useEffect Hook ` +

2823 - `will not persist between re-renders. ` +

2824 - `If it's only needed by this Hook, move the variable inside it. ` +

2825 - `Alternatively, declare a ref with the useRef Hook, ` +

2826 - `and keep the mutable value in its 'current' property.`,

2843 + `Assignments to the 'value2' variable from inside React Hook useEffect ` +

2844 + `will be lost after each render. To preserve the value over time, ` +

2845 + `store it in a useRef Hook and keep the mutable value in the '.current' property. ` +

2846 + `Otherwise, you can move this variable directly inside useEffect.`,

2847 // value

2828 - `Assignments to the 'value' variable from inside a React useEffect Hook ` +

2829 - `will not persist between re-renders. ` +

2830 - `If it's only needed by this Hook, move the variable inside it. ` +

2831 - `Alternatively, declare a ref with the useRef Hook, ` +

2832 - `and keep the mutable value in its 'current' property.`,

2848 + `Assignments to the 'value' variable from inside React Hook useEffect ` +

2849 + `will be lost after each render. To preserve the value over time, ` +

2850 + `store it in a useRef Hook and keep the mutable value in the '.current' property. ` +

2851 + `Otherwise, you can move this variable directly inside useEffect.`,

2852 // value4

2834 - `Assignments to the 'value4' variable from inside a React useEffect Hook ` +

2835 - `will not persist between re-renders. ` +

2836 - `If it's only needed by this Hook, move the variable inside it. ` +

2837 - `Alternatively, declare a ref with the useRef Hook, ` +

2838 - `and keep the mutable value in its 'current' property.`,

2853 + `Assignments to the 'value4' variable from inside React Hook useEffect ` +

2854 + `will be lost after each render. To preserve the value over time, ` +

2855 + `store it in a useRef Hook and keep the mutable value in the '.current' property. ` +

2856 + `Otherwise, you can move this variable directly inside useEffect.`,

2857 // asyncValue

2840 - `Assignments to the 'asyncValue' variable from inside a React useEffect Hook ` +

2841 - `will not persist between re-renders. ` +

2842 - `If it's only needed by this Hook, move the variable inside it. ` +

2843 - `Alternatively, declare a ref with the useRef Hook, ` +

2844 - `and keep the mutable value in its 'current' property.`,

2858 + `Assignments to the 'asyncValue' variable from inside React Hook useEffect ` +

2859 + `will be lost after each render. To preserve the value over time, ` +

2860 + `store it in a useRef Hook and keep the mutable value in the '.current' property. ` +

2861 + `Otherwise, you can move this variable directly inside useEffect.`,

2862 ],

2863 },

2864 {

@@ -2886,23 +2903,20 @@ const tests = {

2903 `,

2904 errors: [

2905 // value

2889 - `Assignments to the 'value' variable from inside a React useEffect Hook ` +

2890 - `will not persist between re-renders. ` +

2891 - `If it's only needed by this Hook, move the variable inside it. ` +

2892 - `Alternatively, declare a ref with the useRef Hook, ` +

2893 - `and keep the mutable value in its 'current' property.`,

2906 + `Assignments to the 'value' variable from inside React Hook useEffect ` +

2907 + `will be lost after each render. To preserve the value over time, ` +

2908 + `store it in a useRef Hook and keep the mutable value in the '.current' property. ` +

2909 + `Otherwise, you can move this variable directly inside useEffect.`,

2910 // value2

2895 - `Assignments to the 'value2' variable from inside a React useEffect Hook ` +

2896 - `will not persist between re-renders. ` +

2897 - `If it's only needed by this Hook, move the variable inside it. ` +

2898 - `Alternatively, declare a ref with the useRef Hook, ` +

2899 - `and keep the mutable value in its 'current' property.`,

2911 + `Assignments to the 'value2' variable from inside React Hook useEffect ` +

2912 + `will be lost after each render. To preserve the value over time, ` +

2913 + `store it in a useRef Hook and keep the mutable value in the '.current' property. ` +

2914 + `Otherwise, you can move this variable directly inside useEffect.`,

2915 // asyncValue

2901 - `Assignments to the 'asyncValue' variable from inside a React useEffect Hook ` +

2902 - `will not persist between re-renders. ` +

2903 - `If it's only needed by this Hook, move the variable inside it. ` +

2904 - `Alternatively, declare a ref with the useRef Hook, ` +

2905 - `and keep the mutable value in its 'current' property.`,

2916 + `Assignments to the 'asyncValue' variable from inside React Hook useEffect ` +

2917 + `will be lost after each render. To preserve the value over time, ` +

2918 + `store it in a useRef Hook and keep the mutable value in the '.current' property. ` +

2919 + `Otherwise, you can move this variable directly inside useEffect.`,

2920 ],

2921 },

2922 {

@@ -2929,11 +2943,10 @@ const tests = {

2943 }

2944 `,

2945 errors: [

2932 - `Accessing 'myRef.current' during the effect cleanup ` +

2933 - `will likely read a different ref value because by this time React ` +

2934 - `has already updated the ref. If this ref is managed by React, store ` +

2935 - `'myRef.current' in a variable inside ` +

2936 - `the effect itself and refer to that variable from the cleanup function.`,

2946 + `The ref value 'myRef.current' will likely have changed by the time ` +

2947 + `this effect cleanup function runs. If this ref points to a node ` +

2948 + `rendered by React, copy 'myRef.current' to a variable inside the effect, ` +

2949 + `and use that variable in the cleanup function.`,

2950 ],

2951 },

2952 {

@@ -2956,11 +2969,10 @@ const tests = {

2969 }

2970 `,

2971 errors: [

2959 - `Accessing 'myRef.current' during the effect cleanup ` +

2960 - `will likely read a different ref value because by this time React ` +

2961 - `has already updated the ref. If this ref is managed by React, store ` +

2962 - `'myRef.current' in a variable inside ` +

2963 - `the effect itself and refer to that variable from the cleanup function.`,

2972 + `The ref value 'myRef.current' will likely have changed by the time ` +

2973 + `this effect cleanup function runs. If this ref points to a node ` +

2974 + `rendered by React, copy 'myRef.current' to a variable inside the effect, ` +

2975 + `and use that variable in the cleanup function.`,

2976 ],

2977 },

2978 {

@@ -2995,11 +3007,10 @@ const tests = {

3007 }

3008 `,

3009 errors: [

2998 - `Accessing 'myRef.current' during the effect cleanup ` +

2999 - `will likely read a different ref value because by this time React ` +

3000 - `has already updated the ref. If this ref is managed by React, store ` +

3001 - `'myRef.current' in a variable inside ` +

3002 - `the effect itself and refer to that variable from the cleanup function.`,

3010 + `The ref value 'myRef.current' will likely have changed by the time ` +

3011 + `this effect cleanup function runs. If this ref points to a node ` +

3012 + `rendered by React, copy 'myRef.current' to a variable inside the effect, ` +

3013 + `and use that variable in the cleanup function.`,

3014 ],

3015 },

3016 {

@@ -3034,11 +3045,10 @@ const tests = {

3045 }

3046 `,

3047 errors: [

3037 - `Accessing 'myRef.current' during the effect cleanup ` +

3038 - `will likely read a different ref value because by this time React ` +

3039 - `has already updated the ref. If this ref is managed by React, store ` +

3040 - `'myRef.current' in a variable inside ` +

3041 - `the effect itself and refer to that variable from the cleanup function.`,

3048 + `The ref value 'myRef.current' will likely have changed by the time ` +

3049 + `this effect cleanup function runs. If this ref points to a node ` +

3050 + `rendered by React, copy 'myRef.current' to a variable inside the effect, ` +

3051 + `and use that variable in the cleanup function.`,

3052 ],

3053 },

3054 {

@@ -3088,7 +3098,7 @@ const tests = {

3098 "React Hook useEffect has an unnecessary dependency: 'window'. " +

3099 'Either exclude it or remove the dependency array. ' +

3100 "Outer scope values like 'window' aren't valid dependencies " +

3091 - "because their mutation doesn't re-render the component.",

3101 + "because mutating them doesn't re-render the component.",

3102 ],

3103 },

3104 {

@@ -3114,7 +3124,7 @@ const tests = {

3124 "React Hook useEffect has an unnecessary dependency: 'MutableStore.hello'. " +

3125 'Either exclude it or remove the dependency array. ' +

3126 "Outer scope values like 'MutableStore.hello' aren't valid dependencies " +

3117 - "because their mutation doesn't re-render the component.",

3127 + "because mutating them doesn't re-render the component.",

3128 ],

3129 },

3130 {

@@ -3151,7 +3161,7 @@ const tests = {

3161 "'MutableStore.hello.world', 'global.stuff', and 'z'. " +

3162 'Either exclude them or remove the dependency array. ' +

3163 "Outer scope values like 'MutableStore.hello.world' aren't valid dependencies " +

3154 - "because their mutation doesn't re-render the component.",

3164 + "because mutating them doesn't re-render the component.",

3165 ],

3166 },

3167 {

@@ -3190,7 +3200,7 @@ const tests = {

3200 "'MutableStore.hello.world', 'global.stuff', and 'z'. " +

3201 'Either exclude them or remove the dependency array. ' +

3202 "Outer scope values like 'MutableStore.hello.world' aren't valid dependencies " +

3193 - "because their mutation doesn't re-render the component.",

3203 + "because mutating them doesn't re-render the component.",

3204 ],

3205 },

3206 {

@@ -3227,7 +3237,7 @@ const tests = {

3237 "'MutableStore.hello.world', 'global.stuff', 'props.foo', 'x', 'y', and 'z'. " +

3238 'Either exclude them or remove the dependency array. ' +

3239 "Outer scope values like 'MutableStore.hello.world' aren't valid dependencies " +

3230 - "because their mutation doesn't re-render the component.",

3240 + "because mutating them doesn't re-render the component.",

3241 ],

3242 },

3243 {

@@ -3956,8 +3966,8 @@ const tests = {

3966 errors: [

3967 "React Hook useEffect has a missing dependency: 'count'. " +

3968 'Either include it or remove the dependency array. ' +

3959 - `You can also write 'setCount(c => ...)' if you ` +

3960 - `only use 'count' for the 'setCount' call.`,

3969 + `You can also do a functional update 'setCount(c => ...)' if you ` +

3970 + `only need 'count' in the 'setCount' call.`,

3971 ],

3972 },

3973 {

@@ -3994,8 +4004,8 @@ const tests = {

4004 errors: [

4005 "React Hook useEffect has missing dependencies: 'count' and 'increment'. " +

4006 'Either include them or remove the dependency array. ' +

3997 - `You can also write 'setCount(c => ...)' if you ` +

3998 - `only use 'count' for the 'setCount' call.`,

4007 + `You can also do a functional update 'setCount(c => ...)' if you ` +

4008 + `only need 'count' in the 'setCount' call.`,

4009 ],

4010 },

4011 {

@@ -4196,8 +4206,8 @@ const tests = {

4206 errors: [

4207 "React Hook useEffect has a missing dependency: 'increment'. " +

4208 'Either include it or remove the dependency array. ' +

4199 - 'You can also replace useState with an inline useReducer ' +

4200 - `if 'setCount' needs the current value of 'increment'.`,

4209 + `If 'setCount' needs the current value of 'increment', ` +

4210 + `you can also switch to useReducer instead of useState and read 'increment' in the reducer.`,

4211 ],

4212 },

4213 {

@@ -4292,7 +4302,7 @@ const tests = {

4302 errors: [

4303 `React Hook useEffect has a missing dependency: 'fetchPodcasts'. ` +

4304 `Either include it or remove the dependency array. ` +

4295 - `If specifying 'fetchPodcasts' makes the dependencies change too often, ` +

4305 + `If 'fetchPodcasts' changes too often, ` +

4306 `find the parent component that defines it and wrap that definition in useCallback.`,

4307 ],

4308 },

@@ -4316,7 +4326,7 @@ const tests = {

4326 errors: [

4327 `React Hook useEffect has a missing dependency: 'fetchPodcasts'. ` +

4328 `Either include it or remove the dependency array. ` +

4319 - `If specifying 'fetchPodcasts' makes the dependencies change too often, ` +

4329 + `If 'fetchPodcasts' changes too often, ` +

4330 `find the parent component that defines it and wrap that definition in useCallback.`,

4331 ],

4332 },

@@ -4348,7 +4358,7 @@ const tests = {

4358 errors: [

4359 `React Hook useEffect has missing dependencies: 'fetchPodcasts' and 'fetchPodcasts2'. ` +

4360 `Either include them or remove the dependency array. ` +

4351 - `If specifying 'fetchPodcasts' makes the dependencies change too often, ` +

4361 + `If 'fetchPodcasts' changes too often, ` +

4362 `find the parent component that defines it and wrap that definition in useCallback.`,

4363 ],

4364 },

@@ -4374,7 +4384,7 @@ const tests = {

4384 errors: [

4385 `React Hook useEffect has a missing dependency: 'fetchPodcasts'. ` +

4386 `Either include it or remove the dependency array. ` +

4377 - `If specifying 'fetchPodcasts' makes the dependencies change too often, ` +

4387 + `If 'fetchPodcasts' changes too often, ` +

4388 `find the parent component that defines it and wrap that definition in useCallback.`,

4389 ],

4390 },

@@ -4426,7 +4436,7 @@ const tests = {

4436 ` }\n` +

4437 `\n` +

4438 ` return () => { ignore = true; };\n` +

4429 - `}, []);\n` +

4439 + `}, ...);\n` +

4440 `\n` +

4441 `This lets you handle multiple requests without bugs.`,

4442 ],

packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js

+47 -43

@@ -94,13 +94,13 @@ export default {

94 reactiveHookName === 'useMemo' ||

95 reactiveHookName === 'useCallback'

96 ) {

97 + // TODO: Can this have an autofix?

98 context.report({

99 node: node.parent.callee,

100 message:

100 - `React Hook ${reactiveHookName} doesn't serve any purpose ` +

101 - `without a dependency array. To enable ` +

102 - `this optimization, pass an array of values used by the ` +

103 - `inner function as the second argument to ${reactiveHookName}.`,

101 + `React Hook ${reactiveHookName} does nothing when called with ` +

102 + `only one argument. Did you forget to pass an array of ` +

103 + `dependencies?`,

104 });

105 }

106 return;

@@ -122,7 +122,7 @@ export default {

122 ` }\n` +

123 `\n` +

124 ` return () => { ignore = true; };\n` +

125 - `}, []);\n` +

125 + `}, ...);\n` +

126 `\n` +

127 `This lets you handle multiple requests without bugs.`,

128 });

@@ -453,11 +453,11 @@ export default {

453 context.report({

454 node: dependencyNode.parent.property,

455 message:

456 - `Accessing '${dependency}.current' during the effect cleanup ` +

457 - `will likely read a different ref value because by this time React ` +

458 - `has already updated the ref. If this ref is managed by React, store ` +

459 - `'${dependency}.current' in a variable inside ` +

460 - `the effect itself and refer to that variable from the cleanup function.`,

456 + `The ref value '${dependency}.current' will likely have ` +

457 + `changed by the time this effect cleanup function runs. If ` +

458 + `this ref points to a node rendered by React, copy ` +

459 + `'${dependency}.current' to a variable inside the effect, and ` +

460 + `use that variable in the cleanup function.`,

461 });

462 },

463 );

@@ -471,9 +471,10 @@ export default {

471 context.report({

472 node: declaredDependenciesNode,

473 message:

474 - `React Hook ${context.getSource(reactiveHook)} has a second ` +

475 - "argument which is not an array literal. This means we can't " +

476 - "statically verify whether you've passed the correct dependencies.",

474 + `React Hook ${context.getSource(reactiveHook)} was passed a ` +

475 + 'dependency list that is not an array literal. This means we ' +

476 + "can't statically verify whether you've passed the correct " +

477 + 'dependencies.',

478 });

479 } else {

480 declaredDependenciesNode.elements.forEach(declaredDependencyNode => {

@@ -501,15 +502,15 @@ export default {

502 } catch (error) {

503 if (/Unsupported node type/.test(error.message)) {

504 if (declaredDependencyNode.type === 'Literal') {

504 - if (typeof declaredDependencyNode.value === 'string') {

505 + if (dependencies.has(declaredDependencyNode.value)) {

506 context.report({

507 node: declaredDependencyNode,

508 message:

509 `The ${

510 declaredDependencyNode.raw

510 - } string literal is not a valid dependency ` +

511 - `because it never changes. Did you mean to ` +

512 - `include ${

511 + } literal is not a valid dependency ` +

512 + `because it never changes. ` +

513 + `Did you mean to include ${

514 declaredDependencyNode.value

515 } in the array instead?`,

516 });

@@ -517,9 +518,9 @@ export default {

518 context.report({

519 node: declaredDependencyNode,

520 message:

520 - `The '${

521 + `The ${

522 declaredDependencyNode.raw

522 - }' literal is not a valid dependency ` +

523 + } literal is not a valid dependency ` +

524 'because it never changes. You can safely remove it.',

525 });

526 }

@@ -570,13 +571,12 @@ export default {

571 context.report({

572 node: writeExpr,

573 message:

573 - `Assignments to the '${key}' variable from inside a React ${context.getSource(

574 - reactiveHook,

575 - )} Hook ` +

576 - `will not persist between re-renders. ` +

577 - `If it's only needed by this Hook, move the variable inside it. ` +

578 - `Alternatively, declare a ref with the useRef Hook, ` +

579 - `and keep the mutable value in its 'current' property.`,

574 + `Assignments to the '${key}' variable from inside React Hook ` +

575 + `${context.getSource(reactiveHook)} will be lost after each ` +

576 + `render. To preserve the value over time, store it in a useRef ` +

577 + `Hook and keep the mutable value in the '.current' property. ` +

578 + `Otherwise, you can move this variable directly inside ` +

579 + `${context.getSource(reactiveHook)}.`,

580 });

581 }

582

@@ -644,7 +644,10 @@ export default {

644 fn.name.name

645 }' definition into its own useCallback() Hook.`;

646 }

647 + // TODO: What if the function needs to change on every render anyway?

648 + // Should we suggest removing effect deps as an appropriate fix too?

649 context.report({

650 + // TODO: Why not report this at the dependency site?

651 node: fn.node,

652 message,

653 fix(fixer) {

@@ -654,10 +657,10 @@ export default {

657 return [

658 // TODO: also add an import?

659 fixer.insertTextBefore(fn.node.init, 'useCallback('),

657 - // TODO: ideally we'd gather deps here but it would

658 - // require restructuring the rule code. For now,

659 - // this is fine. Note we're intentionally not adding

660 - // [] because that changes semantics.

660 + // TODO: ideally we'd gather deps here but it would require

661 + // restructuring the rule code. This will cause a new lint

662 + // error to appear immediately for useCallback. Note we're

663 + // not adding [] because would that changes semantics.

664 fixer.insertTextAfter(fn.node.init, ')'),

665 ];

666 }

@@ -731,7 +734,7 @@ export default {

734 if (badRef !== null) {

735 extraWarning =

736 ` Mutable values like '${badRef}' aren't valid dependencies ` +

734 - "because their mutation doesn't re-render the component.";

737 + "because mutating them doesn't re-render the component.";

738 } else if (externalDependencies.size > 0) {

739 const dep = Array.from(externalDependencies)[0];

740 // Don't show this warning for things that likely just got moved *inside* the callback

@@ -739,7 +742,7 @@ export default {

742 if (!scope.set.has(dep)) {

743 extraWarning =

744 ` Outer scope values like '${dep}' aren't valid dependencies ` +

742 - `because their mutation doesn't re-render the component.`;

745 + `because mutating them doesn't re-render the component.`;

746 }

747 }

748 }

@@ -779,9 +782,10 @@ export default {

782 }

783 if (isPropsOnlyUsedInMembers) {

784 extraWarning =

782 - ` However, the preferred fix is to destructure the 'props' ` +

783 - `object outside of the ${reactiveHookName} call and ` +

784 - `refer to specific props directly by their names.`;

785 + ` However, 'props' will change when *any* prop changes, so the ` +

786 + `preferred fix is to destructure the 'props' object outside of ` +

787 + `the ${reactiveHookName} call and refer to those specific props ` +

788 + `inside ${context.getSource(reactiveHook)}.`;

789 }

790 }

791

@@ -829,8 +833,7 @@ export default {

833 });

834 if (missingCallbackDep !== null) {

835 extraWarning =

832 - ` If specifying '${missingCallbackDep}'` +

833 - ` makes the dependencies change too often, ` +

836 + ` If '${missingCallbackDep}' changes too often, ` +

837 `find the parent component that defines it ` +

838 `and wrap that definition in useCallback.`;

839 }

@@ -906,20 +909,21 @@ export default {

909 break;

910 case 'inlineReducer':

911 extraWarning =

909 - ` You can also replace useState with an inline useReducer ` +

910 - `if '${setStateRecommendation.setter}' needs the ` +

911 - `current value of '${setStateRecommendation.missingDep}'.`;

912 + ` If '${setStateRecommendation.setter}' needs the ` +

913 + `current value of '${setStateRecommendation.missingDep}', ` +

914 + `you can also switch to useReducer instead of useState and ` +

915 + `read '${setStateRecommendation.missingDep}' in the reducer.`;

916 break;

917 case 'updater':

918 extraWarning =

915 - ` You can also write '${

919 + ` You can also do a functional update '${

920 setStateRecommendation.setter

921 }(${setStateRecommendation.missingDep.substring(

922 0,

923 1,

920 - )} => ...)' if you only use '${

924 + )} => ...)' if you only need '${

925 setStateRecommendation.missingDep

922 - }'` + ` for the '${setStateRecommendation.setter}' call.`;

926 + }'` + ` in the '${setStateRecommendation.setter}' call.`;

927 break;

928 default:

929 throw new Error('Unknown case.');