diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 5c3c64c173fa9..2f9764a74db19 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1630,7 +1630,7 @@ const tests = { }, 1000); return () => clearInterval(id); }, [setCount]); - + return

{count}

; } `, @@ -7630,6 +7630,24 @@ const tests = { ], }; +if (__EXPERIMENTAL__) { + tests.valid = [ + ...tests.valid, + { + code: normalizeIndent` + function MyComponent({ theme }) { + const onStuff = useEvent(() => { + showNotification(theme); + }); + useEffect(() => { + onStuff(); + }, []); + } + `, + }, + ]; +} + // Tests that are only valid/invalid across parsers supporting Flow const testsFlow = { valid: [ diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index ee220fa5dba3c..16bec9eb588a9 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -449,7 +449,7 @@ const tests = { }, { code: ` - // This is a false positive (it's valid) that unfortunately + // This is a false positive (it's valid) that unfortunately // we cannot avoid. Prefer to rename it to not start with "use" class Foo extends Component { render() { @@ -974,6 +974,154 @@ const tests = { ], }; +if (__EXPERIMENTAL__) { + tests.valid = [ + ...tests.valid, + ` + // Valid because functions created with useEvent can be called in a useEffect. + function MyComponent({ theme }) { + const onClick = useEvent(() => { + showNotification(theme); + }); + useEffect(() => { + onClick(); + }); + } + `, + ` + // Valid because functions created with useEvent can be called in closures. + function MyComponent({ theme }) { + const onClick = useEvent(() => { + showNotification(theme); + }); + return onClick()}>; + } + `, + ` + // Valid because functions created with useEvent can be called in closures. + function MyComponent({ theme }) { + const onClick = useEvent(() => { + showNotification(theme); + }); + const onClick2 = () => { onClick() }; + const onClick3 = useCallback(() => onClick(), []); + return <> + + + ; + } + `, + ` + // Valid because functions created with useEvent can be passed by reference in useEffect + // and useEvent. + function MyComponent({ theme }) { + const onClick = useEvent(() => { + showNotification(theme); + }); + const onClick2 = useEvent(() => { + debounce(onClick); + }); + useEffect(() => { + let id = setInterval(onClick, 100); + return () => clearInterval(onClick); + }, []); + return onClick2()} /> + } + `, + ` + const MyComponent = ({theme}) => { + const onClick = useEvent(() => { + showNotification(theme); + }); + return onClick()}>; + }; + `, + ` + function MyComponent({ theme }) { + const notificationService = useNotifications(); + const showNotification = useEvent((text) => { + notificationService.notify(theme, text); + }); + const onClick = useEvent((text) => { + showNotification(text); + }); + return onClick(text)} /> + } + `, + ` + function MyComponent({ theme }) { + useEffect(() => { + onClick(); + }); + const onClick = useEvent(() => { + showNotification(theme); + }); + } + `, + ]; + tests.invalid = [ + ...tests.invalid, + { + code: ` + function MyComponent({ theme }) { + const onClick = useEvent(() => { + showNotification(theme); + }); + return ; + } + `, + errors: [useEventError('onClick')], + }, + { + code: ` + // This should error even though it shares an identifier name with the below + function MyComponent({theme}) { + const onClick = useEvent(() => { + showNotification(theme) + }); + return + } + + // The useEvent function shares an identifier name with the above + function MyOtherComponent({theme}) { + const onClick = useEvent(() => { + showNotification(theme) + }); + return onClick()} /> + } + `, + errors: [{...useEventError('onClick'), line: 4}], + }, + { + code: ` + const MyComponent = ({ theme }) => { + const onClick = useEvent(() => { + showNotification(theme); + }); + return ; + } + `, + errors: [useEventError('onClick')], + }, + { + code: ` + // Invalid because onClick is being aliased to foo but not invoked + function MyComponent({ theme }) { + const onClick = useEvent(() => { + showNotification(theme); + }); + let foo; + useEffect(() => { + foo = onClick; + }); + return + } + `, + errors: [useEventError('onClick')], + }, + ]; +} + function conditionalError(hook, hasPreviousFinalizer = false) { return { message: @@ -1031,6 +1179,14 @@ function classError(hook) { }; } +function useEventError(fn) { + return { + message: + `\`${fn}\` 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.', + }; +} + // For easier local testing if (!process.env.CI) { let only = []; diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index c2341886872c1..2ebaf21c62c30 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -157,6 +157,8 @@ export default { // ^^^ true for this reference // const ref = useRef() // ^^^ true for this reference + // const onStuff = useEvent(() => {}) + // ^^^ true for this reference // False for everything else. function isStableKnownHookValue(resolved) { if (!isArray(resolved.defs)) { @@ -223,6 +225,9 @@ export default { if (name === 'useRef' && id.type === 'Identifier') { // useRef() return value is stable. return true; + } else if (isUseEventIdentifier(callee) && id.type === 'Identifier') { + // useEvent() return value is stable. + return true; } else if (name === 'useState' || name === 'useReducer') { // Only consider second value in initializing tuple stable. if ( @@ -1819,3 +1824,10 @@ function isSameIdentifier(a, b) { function isAncestorNodeOf(a, b) { return a.range[0] <= b.range[0] && a.range[1] >= b.range[1]; } + +function isUseEventIdentifier(node) { + if (__EXPERIMENTAL__) { + return node.type === 'Identifier' && node.name === 'useEvent'; + } + return false; +} diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 1957d3ce199af..20ceb24244a9e 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -100,6 +100,13 @@ function isInsideComponentOrHook(node) { return false; } +function isUseEventIdentifier(node) { + if (__EXPERIMENTAL__) { + return node.type === 'Identifier' && node.name === 'useEvent'; + } + return false; +} + export default { meta: { type: 'problem', @@ -110,8 +117,45 @@ export default { }, }, create(context) { + 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); + } + } + } + } + + // 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), @@ -522,6 +566,64 @@ 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 ( + node.callee.type === 'Identifier' && + (node.callee.name === 'useEffect' || + isUseEventIdentifier(node.callee)) && + node.arguments.length > 0 + ) { + // Denote that we have traversed into a useEffect call, and stash the CallExpr for + // comparison later when we exit + lastEffect = node; + } + }, + + Identifier(node) { + // OK - useEffect(() => { setInterval(onClick, ...) }, []); + if (lastEffect != null && node.parent.type === 'CallExpression') { + resolveUseEventViolation(context.getScope(), node); + } + }, + + 'CallExpression:exit'(node) { + if (node === lastEffect) { + lastEffect = null; + } + }, + + FunctionDeclaration(node) { + // function MyComponent() { const onClick = useEvent(...) } + if (isInsideComponentOrHook(node)) { + addAllUseEventViolations(node); + } + }, + + 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.', + }); + } }, }; },