[useEvent] Lint for presence of useEvent functions in dependency lists (#25512)
* [useEvent] Lint for presence of useEvent functions in dependency lists With #25473, the identity of useEvent's return value is no longer stable across renders. Previously, the ExhaustiveDeps lint rule would only allow the omission of the useEvent function, but you could still add it as a dependency. This PR updates the ExhaustiveDeps rule to explicitly check for the presence of useEvent functions in dependency lists, and emits a warning and suggestion/autofixer for removing the dependency.
lauren committed
Oct 19, 2022 at 12:03 UTC
9336e29d917f286f74fc1e10db4cfbc9ccf67a38
2 files changed
+74
-10
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+47
-9
@@ -7635,15 +7635,53 @@ if (__EXPERIMENTAL__) {
7635
...tests.valid,
7636
{
7637
code: normalizeIndent`
7638
- function MyComponent({ theme }) {
7639
- const onStuff = useEvent(() => {
7640
- showNotification(theme);
7641
- });
7642
- useEffect(() => {
7643
- onStuff();
7644
- }, []);
7645
- }
7646
- `,
7638
+ function MyComponent({ theme }) {
7639
+ const onStuff = useEvent(() => {
7640
+ showNotification(theme);
7641
+ });
7642
+ useEffect(() => {
7643
+ onStuff();
7644
+ }, []);
7645
+ }
7646
+ `,
7647
+ },
7648
+ ];
7649
+
7650
+ tests.invalid = [
7651
+ ...tests.invalid,
7652
+ {
7653
+ code: normalizeIndent`
7654
+ function MyComponent({ theme }) {
7655
+ const onStuff = useEvent(() => {
7656
+ showNotification(theme);
7657
+ });
7658
+ useEffect(() => {
7659
+ onStuff();
7660
+ }, [onStuff]);
7661
+ }
7662
+ `,
7663
+ errors: [
7664
+ {
7665
+ message:
7666
+ 'Functions returned from `useEvent` must not be included in the dependency array. ' +
7667
+ 'Remove `onStuff` from the list.',
7668
+ suggestions: [
7669
+ {
7670
+ desc: 'Remove the dependency `onStuff`',
7671
+ output: normalizeIndent`
7672
+ function MyComponent({ theme }) {
7673
+ const onStuff = useEvent(() => {
7674
+ showNotification(theme);
7675
+ });
7676
+ useEffect(() => {
7677
+ onStuff();
7678
+ }, []);
7679
+ }
7680
+ `,
7681
+ },
7682
+ ],
7683
+ },
7684
+ ],
7685
},
7686
];
7687
}
packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+27
-1
@@ -74,6 +74,7 @@ export default {
74
const stateVariables = new WeakSet();
75
const stableKnownValueCache = new WeakMap();
76
const functionWithoutCapturedValueCache = new WeakMap();
77
+ const useEventVariables = new WeakSet();
78
function memoizeWithWeakMap(fn, map) {
79
return function(arg) {
80
if (map.has(arg)) {
@@ -226,7 +227,12 @@ export default {
227
// useRef() return value is stable.
228
return true;
229
} else if (isUseEventIdentifier(callee) && id.type === 'Identifier') {
229
- // useEvent() return value is stable.
230
+ for (const ref of resolved.references) {
231
+ if (ref !== id) {
232
+ useEventVariables.add(ref.identifier);
233
+ }
234
+ }
235
+ // useEvent() return value is always unstable.
236
return true;
237
} else if (name === 'useState' || name === 'useReducer') {
238
// Only consider second value in initializing tuple stable.
@@ -639,6 +645,26 @@ export default {
645
});
646
return;
647
}
648
+ if (useEventVariables.has(declaredDependencyNode)) {
649
+ reportProblem({
650
+ node: declaredDependencyNode,
651
+ message:
652
+ 'Functions returned from `useEvent` must not be included in the dependency array. ' +
653
+ `Remove \`${context.getSource(
654
+ declaredDependencyNode,
655
+ )}\` from the list.`,
656
+ suggest: [
657
+ {
658
+ desc: `Remove the dependency \`${context.getSource(
659
+ declaredDependencyNode,
660
+ )}\``,
661
+ fix(fixer) {
662
+ return fixer.removeRange(declaredDependencyNode.range);
663
+ },
664
+ },
665
+ ],
666
+ });
667
+ }
668
// Try to normalize the declared dependency. If we can't then an error
669
// will be thrown. We will catch that error and report an error.
670
let declaredDependency;