Skip to content

Commit c89a836

Browse files
potetogaearon
poteto
and
gaearon
authored
Update RulesOfHooks with useEvent rules (#25285)
This update to the RulesOfHooks rule checks that functions created with `useEvent` can only be invoked in a `useEffect` callback, in another event function, or a closure. They can't be passed down directly as a reference to child components. This PR also updates the ExhaustiveDeps lint rule to treat useEvent's return value as stable, so it can be omitted from dependency lists. Currently this all gated behind an experimental flag. Co-authored-by: Dan Abramov <dan.abramov@gmail.com>
1 parent efc6a08 commit c89a836

File tree

4 files changed

+290
-2
lines changed

4 files changed

+290
-2
lines changed

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1630,7 +1630,7 @@ const tests = {
16301630
}, 1000);
16311631
return () => clearInterval(id);
16321632
}, [setCount]);
1633-
1633+
16341634
return <h1>{count}</h1>;
16351635
}
16361636
`,
@@ -7630,6 +7630,24 @@ const tests = {
76307630
],
76317631
};
76327632

7633+
if (__EXPERIMENTAL__) {
7634+
tests.valid = [
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+
`,
7647+
},
7648+
];
7649+
}
7650+
76337651
// Tests that are only valid/invalid across parsers supporting Flow
76347652
const testsFlow = {
76357653
valid: [

‎packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js‎

Lines changed: 157 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ const tests = {
449449
},
450450
{
451451
code: `
452-
// This is a false positive (it's valid) that unfortunately
452+
// This is a false positive (it's valid) that unfortunately
453453
// we cannot avoid. Prefer to rename it to not start with "use"
454454
class Foo extends Component {
455455
render() {
@@ -974,6 +974,154 @@ const tests = {
974974
],
975975
};
976976

977+
if (__EXPERIMENTAL__) {
978+
tests.valid = [
979+
...tests.valid,
980+
`
981+
// Valid because functions created with useEvent can be called in a useEffect.
982+
function MyComponent({ theme }) {
983+
const onClick = useEvent(() => {
984+
showNotification(theme);
985+
});
986+
useEffect(() => {
987+
onClick();
988+
});
989+
}
990+
`,
991+
`
992+
// Valid because functions created with useEvent can be called in closures.
993+
function MyComponent({ theme }) {
994+
const onClick = useEvent(() => {
995+
showNotification(theme);
996+
});
997+
return <Child onClick={() => onClick()}></Child>;
998+
}
999+
`,
1000+
`
1001+
// Valid because functions created with useEvent can be called in closures.
1002+
function MyComponent({ theme }) {
1003+
const onClick = useEvent(() => {
1004+
showNotification(theme);
1005+
});
1006+
const onClick2 = () => { onClick() };
1007+
const onClick3 = useCallback(() => onClick(), []);
1008+
return <>
1009+
<Child onClick={onClick2}></Child>
1010+
<Child onClick={onClick3}></Child>
1011+
</>;
1012+
}
1013+
`,
1014+
`
1015+
// Valid because functions created with useEvent can be passed by reference in useEffect
1016+
// and useEvent.
1017+
function MyComponent({ theme }) {
1018+
const onClick = useEvent(() => {
1019+
showNotification(theme);
1020+
});
1021+
const onClick2 = useEvent(() => {
1022+
debounce(onClick);
1023+
});
1024+
useEffect(() => {
1025+
let id = setInterval(onClick, 100);
1026+
return () => clearInterval(onClick);
1027+
}, []);
1028+
return <Child onClick={() => onClick2()} />
1029+
}
1030+
`,
1031+
`
1032+
const MyComponent = ({theme}) => {
1033+
const onClick = useEvent(() => {
1034+
showNotification(theme);
1035+
});
1036+
return <Child onClick={() => onClick()}></Child>;
1037+
};
1038+
`,
1039+
`
1040+
function MyComponent({ theme }) {
1041+
const notificationService = useNotifications();
1042+
const showNotification = useEvent((text) => {
1043+
notificationService.notify(theme, text);
1044+
});
1045+
const onClick = useEvent((text) => {
1046+
showNotification(text);
1047+
});
1048+
return <Child onClick={(text) => onClick(text)} />
1049+
}
1050+
`,
1051+
`
1052+
function MyComponent({ theme }) {
1053+
useEffect(() => {
1054+
onClick();
1055+
});
1056+
const onClick = useEvent(() => {
1057+
showNotification(theme);
1058+
});
1059+
}
1060+
`,
1061+
];
1062+
tests.invalid = [
1063+
...tests.invalid,
1064+
{
1065+
code: `
1066+
function MyComponent({ theme }) {
1067+
const onClick = useEvent(() => {
1068+
showNotification(theme);
1069+
});
1070+
return <Child onClick={onClick}></Child>;
1071+
}
1072+
`,
1073+
errors: [useEventError('onClick')],
1074+
},
1075+
{
1076+
code: `
1077+
// This should error even though it shares an identifier name with the below
1078+
function MyComponent({theme}) {
1079+
const onClick = useEvent(() => {
1080+
showNotification(theme)
1081+
});
1082+
return <Child onClick={onClick} />
1083+
}
1084+
1085+
// The useEvent function shares an identifier name with the above
1086+
function MyOtherComponent({theme}) {
1087+
const onClick = useEvent(() => {
1088+
showNotification(theme)
1089+
});
1090+
return <Child onClick={() => onClick()} />
1091+
}
1092+
`,
1093+
errors: [{...useEventError('onClick'), line: 4}],
1094+
},
1095+
{
1096+
code: `
1097+
const MyComponent = ({ theme }) => {
1098+
const onClick = useEvent(() => {
1099+
showNotification(theme);
1100+
});
1101+
return <Child onClick={onClick}></Child>;
1102+
}
1103+
`,
1104+
errors: [useEventError('onClick')],
1105+
},
1106+
{
1107+
code: `
1108+
// Invalid because onClick is being aliased to foo but not invoked
1109+
function MyComponent({ theme }) {
1110+
const onClick = useEvent(() => {
1111+
showNotification(theme);
1112+
});
1113+
let foo;
1114+
useEffect(() => {
1115+
foo = onClick;
1116+
});
1117+
return <Bar onClick={foo} />
1118+
}
1119+
`,
1120+
errors: [useEventError('onClick')],
1121+
},
1122+
];
1123+
}
1124+
9771125
function conditionalError(hook, hasPreviousFinalizer = false) {
9781126
return {
9791127
message:
@@ -1031,6 +1179,14 @@ function classError(hook) {
10311179
};
10321180
}
10331181

1182+
function useEventError(fn) {
1183+
return {
1184+
message:
1185+
`\`${fn}\` is a function created with React Hook "useEvent", and can only be called from ` +
1186+
'the same component. They cannot be assigned to variables or passed down.',
1187+
};
1188+
}
1189+
10341190
// For easier local testing
10351191
if (!process.env.CI) {
10361192
let only = [];

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ export default {
157157
// ^^^ true for this reference
158158
// const ref = useRef()
159159
// ^^^ true for this reference
160+
// const onStuff = useEvent(() => {})
161+
// ^^^ true for this reference
160162
// False for everything else.
161163
function isStableKnownHookValue(resolved) {
162164
if (!isArray(resolved.defs)) {
@@ -223,6 +225,9 @@ export default {
223225
if (name === 'useRef' && id.type === 'Identifier') {
224226
// useRef() return value is stable.
225227
return true;
228+
} else if (isUseEventIdentifier(callee) && id.type === 'Identifier') {
229+
// useEvent() return value is stable.
230+
return true;
226231
} else if (name === 'useState' || name === 'useReducer') {
227232
// Only consider second value in initializing tuple stable.
228233
if (
@@ -1819,3 +1824,10 @@ function isSameIdentifier(a, b) {
18191824
function isAncestorNodeOf(a, b) {
18201825
return a.range[0] <= b.range[0] && a.range[1] >= b.range[1];
18211826
}
1827+
1828+
function isUseEventIdentifier(node) {
1829+
if (__EXPERIMENTAL__) {
1830+
return node.type === 'Identifier' && node.name === 'useEvent';
1831+
}
1832+
return false;
1833+
}

‎packages/eslint-plugin-react-hooks/src/RulesOfHooks.js‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ function isInsideComponentOrHook(node) {
100100
return false;
101101
}
102102

103+
function isUseEventIdentifier(node) {
104+
if (__EXPERIMENTAL__) {
105+
return node.type === 'Identifier' && node.name === 'useEvent';
106+
}
107+
return false;
108+
}
109+
103110
export default {
104111
meta: {
105112
type: 'problem',
@@ -110,8 +117,45 @@ export default {
110117
},
111118
},
112119
create(context) {
120+
let lastEffect = null;
113121
const codePathReactHooksMapStack = [];
114122
const codePathSegmentStack = [];
123+
const useEventViolations = new Set();
124+
125+
// For a given AST node, iterate through the top level statements and add all useEvent
126+
// definitions. We can do this in non-Program nodes because we can rely on the assumption that
127+
// useEvent functions can only be declared within a component or hook at its top level.
128+
function addAllUseEventViolations(node) {
129+
if (node.body.type !== 'BlockStatement') return;
130+
for (const statement of node.body.body) {
131+
if (statement.type !== 'VariableDeclaration') continue;
132+
for (const declaration of statement.declarations) {
133+
if (
134+
declaration.type === 'VariableDeclarator' &&
135+
declaration.init &&
136+
declaration.init.type === 'CallExpression' &&
137+
declaration.init.callee &&
138+
isUseEventIdentifier(declaration.init.callee)
139+
) {
140+
useEventViolations.add(declaration.id);
141+
}
142+
}
143+
}
144+
}
145+
146+
// Resolve a useEvent violation, ie the useEvent created function was called.
147+
function resolveUseEventViolation(scope, ident) {
148+
if (scope.references == null || useEventViolations.size === 0) return;
149+
for (const ref of scope.references) {
150+
if (ref.resolved == null) continue;
151+
const [useEventFunctionIdentifier] = ref.resolved.identifiers;
152+
if (ident.name === useEventFunctionIdentifier.name) {
153+
useEventViolations.delete(useEventFunctionIdentifier);
154+
break;
155+
}
156+
}
157+
}
158+
115159
return {
116160
// Maintain code segment path stack as we traverse.
117161
onCodePathSegmentStart: segment => codePathSegmentStack.push(segment),
@@ -522,6 +566,64 @@ export default {
522566
}
523567
reactHooks.push(node.callee);
524568
}
569+
570+
const scope = context.getScope();
571+
// useEvent: Resolve a function created with useEvent that is invoked locally at least once.
572+
// OK - onClick();
573+
resolveUseEventViolation(scope, node.callee);
574+
575+
// useEvent: useEvent functions can be passed by reference within useEffect as well as in
576+
// another useEvent
577+
if (
578+
node.callee.type === 'Identifier' &&
579+
(node.callee.name === 'useEffect' ||
580+
isUseEventIdentifier(node.callee)) &&
581+
node.arguments.length > 0
582+
) {
583+
// Denote that we have traversed into a useEffect call, and stash the CallExpr for
584+
// comparison later when we exit
585+
lastEffect = node;
586+
}
587+
},
588+
589+
Identifier(node) {
590+
// OK - useEffect(() => { setInterval(onClick, ...) }, []);
591+
if (lastEffect != null && node.parent.type === 'CallExpression') {
592+
resolveUseEventViolation(context.getScope(), node);
593+
}
594+
},
595+
596+
'CallExpression:exit'(node) {
597+
if (node === lastEffect) {
598+
lastEffect = null;
599+
}
600+
},
601+
602+
FunctionDeclaration(node) {
603+
// function MyComponent() { const onClick = useEvent(...) }
604+
if (isInsideComponentOrHook(node)) {
605+
addAllUseEventViolations(node);
606+
}
607+
},
608+
609+
ArrowFunctionExpression(node) {
610+
// const MyComponent = () => { const onClick = useEvent(...) }
611+
if (isInsideComponentOrHook(node)) {
612+
addAllUseEventViolations(node);
613+
}
614+
},
615+
616+
'Program:exit'(_node) {
617+
for (const node of useEventViolations.values()) {
618+
context.report({
619+
node,
620+
message:
621+
`\`${context.getSource(
622+
node,
623+
)}\` is a function created with React Hook "useEvent", and can only be called from ` +
624+
'the same component. They cannot be assigned to variables or passed down.',
625+
});
626+
}
525627
},
526628
};
527629
},

0 commit comments

Comments
 (0)