More concise messages (#15053)
Dan Abramov committed
Mar 7, 2019 at 15:21 UTC
eb6247a9ab7653aac346db08faf0862c58b055df
2 files changed
+80
-79
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+59
-53
@@ -1236,7 +1236,7 @@ const tests = {
1236
errors: [
1237
"React Hook useCallback has a missing dependency: 'local2'. " +
1238
'Either include it or remove the dependency array. ' +
1239
- "Values like 'local1' aren't valid dependencies " +
1239
+ "Outer scope values like 'local1' aren't valid dependencies " +
1240
"because their mutation doesn't re-render the component.",
1241
],
1242
},
@@ -1302,7 +1302,7 @@ const tests = {
1302
errors: [
1303
"React Hook useCallback has an unnecessary dependency: 'window'. " +
1304
'Either exclude it or remove the dependency array. ' +
1305
- "Values like 'window' aren't valid dependencies " +
1305
+ "Outer scope values like 'window' aren't valid dependencies " +
1306
"because their mutation doesn't re-render the component.",
1307
],
1308
},
@@ -2304,9 +2304,8 @@ const tests = {
2304
errors: [
2305
"React Hook useEffect has a missing dependency: 'state'. " +
2306
'Either include it or remove the dependency array. ' +
2307
- `If 'state' is only necessary for calculating the next state, ` +
2308
- `consider refactoring to the setState(state => ...) form which ` +
2309
- `doesn't need to depend on the state from outside.`,
2307
+ `You can also write 'setState(state => ...)' ` +
2308
+ `if you only use 'state' for the 'setState' call.`,
2309
],
2310
},
2311
{
@@ -2336,9 +2335,8 @@ const tests = {
2335
errors: [
2336
"React Hook useEffect has a missing dependency: 'state'. " +
2337
'Either include it or remove the dependency array. ' +
2339
- `If 'state' is only necessary for calculating the next state, ` +
2340
- `consider refactoring to the setState(state => ...) form which ` +
2341
- `doesn't need to depend on the state from outside.`,
2338
+ `You can also write 'setState(state => ...)' ` +
2339
+ `if you only use 'state' for the 'setState' call.`,
2340
],
2341
},
2342
{
@@ -3066,7 +3064,7 @@ const tests = {
3064
errors: [
3065
"React Hook useEffect has an unnecessary dependency: 'window'. " +
3066
'Either exclude it or remove the dependency array. ' +
3069
- "Values like 'window' aren't valid dependencies " +
3067
+ "Outer scope values like 'window' aren't valid dependencies " +
3068
"because their mutation doesn't re-render the component.",
3069
],
3070
},
@@ -3092,7 +3090,7 @@ const tests = {
3090
errors: [
3091
"React Hook useEffect has an unnecessary dependency: 'MutableStore.hello'. " +
3092
'Either exclude it or remove the dependency array. ' +
3095
- "Values like 'MutableStore.hello' aren't valid dependencies " +
3093
+ "Outer scope values like 'MutableStore.hello' aren't valid dependencies " +
3094
"because their mutation doesn't re-render the component.",
3095
],
3096
},
@@ -3129,7 +3127,7 @@ const tests = {
3127
'React Hook useEffect has unnecessary dependencies: ' +
3128
"'MutableStore.hello.world', 'global.stuff', and 'z'. " +
3129
'Either exclude them or remove the dependency array. ' +
3132
- "Values like 'MutableStore.hello.world' aren't valid dependencies " +
3130
+ "Outer scope values like 'MutableStore.hello.world' aren't valid dependencies " +
3131
"because their mutation doesn't re-render the component.",
3132
],
3133
},
@@ -3168,7 +3166,7 @@ const tests = {
3166
'React Hook useEffect has unnecessary dependencies: ' +
3167
"'MutableStore.hello.world', 'global.stuff', and 'z'. " +
3168
'Either exclude them or remove the dependency array. ' +
3171
- "Values like 'MutableStore.hello.world' aren't valid dependencies " +
3169
+ "Outer scope values like 'MutableStore.hello.world' aren't valid dependencies " +
3170
"because their mutation doesn't re-render the component.",
3171
],
3172
},
@@ -3205,7 +3203,7 @@ const tests = {
3203
'React Hook useCallback has unnecessary dependencies: ' +
3204
"'MutableStore.hello.world', 'global.stuff', 'props.foo', 'x', 'y', and 'z'. " +
3205
'Either exclude them or remove the dependency array. ' +
3208
- "Values like 'MutableStore.hello.world' aren't valid dependencies " +
3206
+ "Outer scope values like 'MutableStore.hello.world' aren't valid dependencies " +
3207
"because their mutation doesn't re-render the component.",
3208
],
3209
},
@@ -3468,8 +3466,7 @@ const tests = {
3466
errors: [
3467
`The 'handleNext' function makes the dependencies of ` +
3468
`useEffect Hook (at line 11) change on every render. ` +
3471
- `To fix this, move the 'handleNext' function ` +
3472
- `inside the useEffect callback (at line 9). Alternatively, ` +
3469
+ `Move it inside the useEffect callback. Alternatively, ` +
3470
`wrap the 'handleNext' definition into its own useCallback() Hook.`,
3471
],
3472
},
@@ -3507,8 +3504,7 @@ const tests = {
3504
errors: [
3505
`The 'handleNext' function makes the dependencies of ` +
3506
`useEffect Hook (at line 11) change on every render. ` +
3510
- `To fix this, move the 'handleNext' function ` +
3511
- `inside the useEffect callback (at line 9). Alternatively, ` +
3507
+ `Move it inside the useEffect callback. Alternatively, ` +
3508
`wrap the 'handleNext' definition into its own useCallback() Hook.`,
3509
],
3510
},
@@ -3605,17 +3601,14 @@ const tests = {
3601
`,
3602
errors: [
3603
"The 'handleNext1' function makes the dependencies of useEffect Hook " +
3608
- "(at line 14) change on every render. To fix this, move the 'handleNext1' " +
3609
- 'function inside the useEffect callback (at line 12). Alternatively, wrap the ' +
3610
- "'handleNext1' definition into its own useCallback() Hook.",
3604
+ '(at line 14) change on every render. Move it inside the useEffect callback. ' +
3605
+ "Alternatively, wrap the 'handleNext1' definition into its own useCallback() Hook.",
3606
"The 'handleNext2' function makes the dependencies of useLayoutEffect Hook " +
3612
- "(at line 17) change on every render. To fix this, move the 'handleNext2' " +
3613
- 'function inside the useLayoutEffect callback (at line 15). Alternatively, wrap the ' +
3614
- "'handleNext2' definition into its own useCallback() Hook.",
3607
+ '(at line 17) change on every render. Move it inside the useLayoutEffect callback. ' +
3608
+ "Alternatively, wrap the 'handleNext2' definition into its own useCallback() Hook.",
3609
"The 'handleNext3' function makes the dependencies of useMemo Hook " +
3616
- "(at line 20) change on every render. To fix this, move the 'handleNext3' " +
3617
- 'function inside the useMemo callback (at line 18). Alternatively, wrap the ' +
3618
- "'handleNext3' definition into its own useCallback() Hook.",
3610
+ '(at line 20) change on every render. Move it inside the useMemo callback. ' +
3611
+ "Alternatively, wrap the 'handleNext3' definition into its own useCallback() Hook.",
3612
],
3613
},
3614
{
@@ -3673,17 +3666,14 @@ const tests = {
3666
`,
3667
errors: [
3668
"The 'handleNext1' function makes the dependencies of useEffect Hook " +
3676
- "(at line 15) change on every render. To fix this, move the 'handleNext1' " +
3677
- 'function inside the useEffect callback (at line 12). Alternatively, wrap the ' +
3678
- "'handleNext1' definition into its own useCallback() Hook.",
3669
+ '(at line 15) change on every render. Move it inside the useEffect callback. ' +
3670
+ "Alternatively, wrap the 'handleNext1' definition into its own useCallback() Hook.",
3671
"The 'handleNext2' function makes the dependencies of useLayoutEffect Hook " +
3680
- "(at line 19) change on every render. To fix this, move the 'handleNext2' " +
3681
- 'function inside the useLayoutEffect callback (at line 16). Alternatively, wrap the ' +
3682
- "'handleNext2' definition into its own useCallback() Hook.",
3672
+ '(at line 19) change on every render. Move it inside the useLayoutEffect callback. ' +
3673
+ "Alternatively, wrap the 'handleNext2' definition into its own useCallback() Hook.",
3674
"The 'handleNext3' function makes the dependencies of useMemo Hook " +
3684
- "(at line 23) change on every render. To fix this, move the 'handleNext3' " +
3685
- 'function inside the useMemo callback (at line 20). Alternatively, wrap the ' +
3686
- "'handleNext3' definition into its own useCallback() Hook.",
3675
+ '(at line 23) change on every render. Move it inside the useMemo callback. ' +
3676
+ "Alternatively, wrap the 'handleNext3' definition into its own useCallback() Hook.",
3677
],
3678
},
3679
{
@@ -3907,8 +3897,7 @@ const tests = {
3897
errors: [
3898
`The 'handleNext' function makes the dependencies of ` +
3899
`useEffect Hook (at line 14) change on every render. ` +
3910
- `To fix this, move the 'handleNext' function inside ` +
3911
- `the useEffect callback (at line 12). Alternatively, wrap the ` +
3900
+ `Move it inside the useEffect callback. Alternatively, wrap the ` +
3901
`'handleNext' definition into its own useCallback() Hook.`,
3902
],
3903
},
@@ -3944,9 +3933,8 @@ const tests = {
3933
errors: [
3934
"React Hook useEffect has a missing dependency: 'count'. " +
3935
'Either include it or remove the dependency array. ' +
3947
- `If 'count' is only necessary for calculating the next state, ` +
3948
- `consider refactoring to the setCount(count => ...) form which ` +
3949
- `doesn't need to depend on the state from outside.`,
3936
+ `You can also write 'setCount(count => ...)' if you ` +
3937
+ `only use 'count' for the 'setCount' call.`,
3938
],
3939
},
3940
{
@@ -3983,9 +3971,8 @@ const tests = {
3971
errors: [
3972
"React Hook useEffect has missing dependencies: 'count' and 'increment'. " +
3973
'Either include them or remove the dependency array. ' +
3986
- `If 'count' is only necessary for calculating the next state, ` +
3987
- `consider refactoring to the setCount(count => ...) form which ` +
3988
- `doesn't need to depend on the state from outside.`,
3974
+ `You can also write 'setCount(count => ...)' if you ` +
3975
+ `only use 'count' for the 'setCount' call.`,
3976
],
3977
},
3978
{
@@ -4022,9 +4009,8 @@ const tests = {
4009
errors: [
4010
"React Hook useEffect has a missing dependency: 'increment'. " +
4011
'Either include it or remove the dependency array. ' +
4025
- `If 'increment' is only necessary for calculating the next state, ` +
4026
- `consider refactoring to the useReducer Hook. This ` +
4027
- `lets you move the calculation of next state outside the effect.`,
4012
+ `You can also replace multiple useState variables with useReducer ` +
4013
+ `if 'setCount' needs the current value of 'increment'.`,
4014
],
4015
},
4016
{
@@ -4150,8 +4136,7 @@ const tests = {
4136
`,
4137
errors: [
4138
`The 'increment' function makes the dependencies of useEffect Hook ` +
4153
- `(at line 14) change on every render. To fix this, move the ` +
4154
- `'increment' function inside the useEffect callback (at line 9). ` +
4139
+ `(at line 14) change on every render. Move it inside the useEffect callback. ` +
4140
`Alternatively, wrap the \'increment\' definition into its own ` +
4141
`useCallback() Hook.`,
4142
],
@@ -4188,11 +4173,8 @@ const tests = {
4173
errors: [
4174
"React Hook useEffect has a missing dependency: 'increment'. " +
4175
'Either include it or remove the dependency array. ' +
4191
- `If 'increment' is only necessary for calculating the next state, ` +
4192
- `consider refactoring to the useReducer Hook. This lets you move ` +
4193
- `the calculation of next state outside the effect. ` +
4194
- `You can then read 'increment' from the reducer ` +
4195
- `by putting it directly in your component.`,
4176
+ 'You can also replace useState with an inline useReducer ' +
4177
+ `if 'setCount' needs the current value of 'increment'.`,
4178
],
4179
},
4180
{
@@ -4373,6 +4355,30 @@ const tests = {
4355
`find the parent component that defines it and wrap that definition in useCallback.`,
4356
],
4357
},
4358
+ {
4359
+ // The mistake here is that it was moved inside the effect
4360
+ // so it can't be referenced in the deps array.
4361
+ code: `
4362
+ function Thing() {
4363
+ useEffect(() => {
4364
+ const fetchData = async () => {};
4365
+ fetchData();
4366
+ }, [fetchData]);
4367
+ }
4368
+ `,
4369
+ output: `
4370
+ function Thing() {
4371
+ useEffect(() => {
4372
+ const fetchData = async () => {};
4373
+ fetchData();
4374
+ }, []);
4375
+ }
4376
+ `,
4377
+ errors: [
4378
+ `React Hook useEffect has an unnecessary dependency: 'fetchData'. ` +
4379
+ `Either exclude it or remove the dependency array.`,
4380
+ ],
4381
+ },
4382
],
4383
};
4384
packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+21
-26
@@ -617,10 +617,7 @@ export default {
617
}' definition into its own useCallback() Hook.`;
618
} else {
619
message +=
620
- ` To fix this, move the '${fn.name.name}' function ` +
621
- `inside the ${reactiveHookName} callback (at line ${
622
- node.loc.start.line
623
- }). ` +
620
+ ` Move it inside the ${reactiveHookName} callback. ` +
621
`Alternatively, wrap the '${
622
fn.name.name
623
}' definition into its own useCallback() Hook.`;
@@ -715,9 +712,13 @@ export default {
712
"because their mutation doesn't re-render the component.";
713
} else if (externalDependencies.size > 0) {
714
const dep = Array.from(externalDependencies)[0];
718
- extraWarning =
719
- ` Values like '${dep}' aren't valid dependencies ` +
720
- `because their mutation doesn't re-render the component.`;
715
+ // Don't show this warning for things that likely just got moved *inside* the callback
716
+ // because in that case they're clearly not referring to globals.
717
+ if (!scope.set.has(dep)) {
718
+ extraWarning =
719
+ ` Outer scope values like '${dep}' aren't valid dependencies ` +
720
+ `because their mutation doesn't re-render the component.`;
721
+ }
722
}
723
}
724
@@ -874,36 +875,30 @@ export default {
875
}
876
});
877
if (setStateRecommendation !== null) {
877
- let suggestion;
878
switch (setStateRecommendation.form) {
879
case 'reducer':
880
- suggestion =
881
- 'useReducer Hook. This lets you move the calculation ' +
882
- 'of next state outside the effect.';
880
+ extraWarning =
881
+ ` You can also replace multiple useState variables with useReducer ` +
882
+ `if '${setStateRecommendation.setter}' needs the ` +
883
+ `current value of '${setStateRecommendation.missingDep}'.`;
884
break;
885
case 'inlineReducer':
885
- suggestion =
886
- 'useReducer Hook. This lets you move the calculation ' +
887
- 'of next state outside the effect. You can then ' +
888
- `read '${
889
- setStateRecommendation.missingDep
890
- }' from the reducer ` +
891
- `by putting it directly in your component.`;
886
+ extraWarning =
887
+ ` You can also replace useState with an inline useReducer ` +
888
+ `if '${setStateRecommendation.setter}' needs the ` +
889
+ `current value of '${setStateRecommendation.missingDep}'.`;
890
break;
891
case 'updater':
894
- suggestion =
895
- `${setStateRecommendation.setter}(${
892
+ extraWarning =
893
+ ` You can also write '${setStateRecommendation.setter}(${
894
setStateRecommendation.missingDep
897
- } => ...) form ` +
898
- `which doesn't need to depend on the state from outside.`;
895
+ } => ...)' if you only use '${
896
+ setStateRecommendation.missingDep
897
+ }'` + ` for the '${setStateRecommendation.setter}' call.`;
898
break;
899
default:
900
throw new Error('Unknown case.');
901
}
903
- extraWarning =
904
- ` If '${setStateRecommendation.missingDep}'` +
905
- ` is only necessary for calculating the next state, ` +
906
- `consider refactoring to the ${suggestion}`;
902
}
903
}
904