diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index 16bec9eb588a9..b077b34edb8e8 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -1090,7 +1090,7 @@ if (__EXPERIMENTAL__) { return onClick()} /> } `, - errors: [{...useEventError('onClick'), line: 4}], + errors: [{...useEventError('onClick'), line: 7}], }, { code: ` @@ -1110,13 +1110,26 @@ if (__EXPERIMENTAL__) { const onClick = useEvent(() => { showNotification(theme); }); - let foo; - useEffect(() => { - foo = onClick; - }); + let foo = onClick; return } `, + errors: [{...useEventError('onClick'), line: 7}], + }, + { + code: ` + // Should error because it's being passed down to JSX, although it's been referenced once + // in an effect + function MyComponent({ theme }) { + const onClick = useEvent(() => { + showNotification(them); + }); + useEffect(() => { + setTimeout(onClick, 100); + }); + return + } + `, errors: [useEventError('onClick')], }, ]; diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 20ceb24244a9e..2164d63aac47b 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -120,42 +120,30 @@ export default { let lastEffect = null; const codePathReactHooksMapStack = []; const codePathSegmentStack = []; - const useEventViolations = new Set(); - - // For a given AST node, iterate through the top level statements and add all useEvent - // definitions. We can do this in non-Program nodes because we can rely on the assumption that - // useEvent functions can only be declared within a component or hook at its top level. - function addAllUseEventViolations(node) { - if (node.body.type !== 'BlockStatement') return; - for (const statement of node.body.body) { - if (statement.type !== 'VariableDeclaration') continue; - for (const declaration of statement.declarations) { - if ( - declaration.type === 'VariableDeclarator' && - declaration.init && - declaration.init.type === 'CallExpression' && - declaration.init.callee && - isUseEventIdentifier(declaration.init.callee) - ) { - useEventViolations.add(declaration.id); + const useEventFunctions = new WeakSet(); + + // For a given scope, iterate through the references and add all useEvent definitions. We can + // do this in non-Program nodes because we can rely on the assumption that useEvent functions + // can only be declared within a component or hook at its top level. + function recordAllUseEventFunctions(scope) { + for (const reference of scope.references) { + const parent = reference.identifier.parent; + if ( + parent.type === 'VariableDeclarator' && + parent.init && + parent.init.type === 'CallExpression' && + parent.init.callee && + isUseEventIdentifier(parent.init.callee) + ) { + for (const ref of reference.resolved.references) { + if (ref !== reference) { + useEventFunctions.add(ref.identifier); + } } } } } - // Resolve a useEvent violation, ie the useEvent created function was called. - function resolveUseEventViolation(scope, ident) { - if (scope.references == null || useEventViolations.size === 0) return; - for (const ref of scope.references) { - if (ref.resolved == null) continue; - const [useEventFunctionIdentifier] = ref.resolved.identifiers; - if (ident.name === useEventFunctionIdentifier.name) { - useEventViolations.delete(useEventFunctionIdentifier); - break; - } - } - } - return { // Maintain code segment path stack as we traverse. onCodePathSegmentStart: segment => codePathSegmentStack.push(segment), @@ -567,11 +555,6 @@ export default { reactHooks.push(node.callee); } - const scope = context.getScope(); - // useEvent: Resolve a function created with useEvent that is invoked locally at least once. - // OK - onClick(); - resolveUseEventViolation(scope, node.callee); - // useEvent: useEvent functions can be passed by reference within useEffect as well as in // another useEvent if ( @@ -587,9 +570,21 @@ export default { }, Identifier(node) { - // OK - useEffect(() => { setInterval(onClick, ...) }, []); - if (lastEffect != null && node.parent.type === 'CallExpression') { - resolveUseEventViolation(context.getScope(), node); + // This identifier resolves to a useEvent function, but isn't being referenced in an + // effect or another event function. It isn't being called either. + if ( + lastEffect == null && + useEventFunctions.has(node) && + node.parent.type !== 'CallExpression' + ) { + context.report({ + node, + message: + `\`${context.getSource( + node, + )}\` is a function created with React Hook "useEvent", and can only be called from ` + + 'the same component. They cannot be assigned to variables or passed down.', + }); } }, @@ -602,27 +597,14 @@ export default { FunctionDeclaration(node) { // function MyComponent() { const onClick = useEvent(...) } if (isInsideComponentOrHook(node)) { - addAllUseEventViolations(node); + recordAllUseEventFunctions(context.getScope()); } }, ArrowFunctionExpression(node) { // const MyComponent = () => { const onClick = useEvent(...) } if (isInsideComponentOrHook(node)) { - addAllUseEventViolations(node); - } - }, - - 'Program:exit'(_node) { - for (const node of useEventViolations.values()) { - context.report({ - node, - message: - `\`${context.getSource( - node, - )}\` is a function created with React Hook "useEvent", and can only be called from ` + - 'the same component. They cannot be assigned to variables or passed down.', - }); + recordAllUseEventFunctions(context.getScope()); } }, };