From ec0e8e876c0ec4b2bd2b87708e973f17bd04de7e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 19 Aug 2022 17:51:51 +0100 Subject: [PATCH 1/6] [ESLint] Treat useEvent retval as stable --- .../__tests__/ESLintRuleExhaustiveDeps-test.js | 12 ++++++++++++ .../eslint-plugin-react-hooks/src/ExhaustiveDeps.js | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 5c3c64c173fa9..3fd1fd9dff227 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1452,6 +1452,18 @@ const tests = { } `, }, + { + code: normalizeIndent` + function MyComponent({ theme }) { + const onStuff = useEvent(() => { + showNotification(theme); + }); + useEffect(() => { + onStuff(); + }, []); + } + `, + }, ], invalid: [ { diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index c2341886872c1..fc388f21a30ef 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 (name === 'useEvent' && 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 ( From 9804d1a428b4f7b0e232610873222be84e776723 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 16 Sep 2022 19:20:59 -0400 Subject: [PATCH 2/6] Check that useEvent is locally called This update to the lint rule checks that functions created with `useEvent` can only be invoked in a `useEffect`callback or closure. They can't be passed down directly as a reference to child components. --- .../__tests__/ESLintRulesOfHooks-test.js | 144 +++++++++++++++++- .../src/ExhaustiveDeps.js | 5 +- .../src/RulesOfHooks.js | 100 ++++++++++++ 3 files changed, 247 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index ee220fa5dba3c..996a7873e9b4f 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -406,6 +406,82 @@ const tests = { const [myState, setMyState] = useState(null); } `, + ` + // 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. + function MyComponent({ theme }) { + const onClick = useEvent(() => { + showNotification(theme); + }); + useEffect(() => { + let id = setInterval(onClick, 100); + return () => clearInterval(onClick); + }, []); + } + `, + ` + 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); + }); + } + `, ], invalid: [ { @@ -449,7 +525,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() { @@ -971,6 +1047,64 @@ const tests = { `, errors: [classError('useState')], }, + { + 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')], + }, ], }; @@ -1031,6 +1165,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 fc388f21a30ef..d0fbf8fbdfb3d 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -225,7 +225,10 @@ export default { if (name === 'useRef' && id.type === 'Identifier') { // useRef() return value is stable. return true; - } else if (name === 'useEvent' && id.type === 'Identifier') { + } else if ( + (name === 'experimental_useEvent' || name === 'useEvent') && + id.type === 'Identifier' + ) { // useEvent() return value is stable. return true; } else if (name === 'useState' || name === 'useReducer') { diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 1957d3ce199af..c9099b567e13b 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) { + return ( + node.type === 'Identifier' && + (node.name === 'useEvent' || node.name === 'experimental_useEvent') + ); +} + 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,62 @@ 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 + if ( + node.callee.type === 'Identifier' && + node.callee.name === 'useEffect' && + 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.', + }); + } }, }; }, From 16e669b2ba694b32696440ba9a8820baa3d10d03 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Thu, 22 Sep 2022 16:36:29 -0400 Subject: [PATCH 3/6] Update ExhaustiveDeps to only check experimental_useEvent --- .../__tests__/ESLintRuleExhaustiveDeps-test.js | 4 ++-- packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 3fd1fd9dff227..2c88fab9cc8d9 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1455,7 +1455,7 @@ const tests = { { code: normalizeIndent` function MyComponent({ theme }) { - const onStuff = useEvent(() => { + const onStuff = experimental_useEvent(() => { showNotification(theme); }); useEffect(() => { @@ -1642,7 +1642,7 @@ const tests = { }, 1000); return () => clearInterval(id); }, [setCount]); - + return

{count}

; } `, diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index d0fbf8fbdfb3d..f496bc8f5e1b5 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -226,7 +226,7 @@ export default { // useRef() return value is stable. return true; } else if ( - (name === 'experimental_useEvent' || name === 'useEvent') && + name === 'experimental_useEvent' && id.type === 'Identifier' ) { // useEvent() return value is stable. From 9550edbb9fc3d85c7cb87d569eddbe0cd6a3a713 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 23 Sep 2022 16:24:20 -0400 Subject: [PATCH 4/6] Gate useEvent handling in ExhaustiveDeps as experimental To keep the test simple, just imperatively only run the test if the EXPERIMENTAL flag is set. --- .../ESLintRuleExhaustiveDeps-test.js | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 2c88fab9cc8d9..b6dd25b57024a 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -1452,18 +1452,6 @@ const tests = { } `, }, - { - code: normalizeIndent` - function MyComponent({ theme }) { - const onStuff = experimental_useEvent(() => { - showNotification(theme); - }); - useEffect(() => { - onStuff(); - }, []); - } - `, - }, ], invalid: [ { @@ -7642,6 +7630,21 @@ const tests = { ], }; +if (__EXPERIMENTAL__) { + tests.valid.push({ + code: normalizeIndent` + function MyComponent({ theme }) { + const onStuff = experimental_useEvent(() => { + showNotification(theme); + }); + useEffect(() => { + onStuff(); + }, []); + } + `, + }); +} + // Tests that are only valid/invalid across parsers supporting Flow const testsFlow = { valid: [ From 76c096c66c82c3d8f19fe7b1950dc3a407dd25da Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 23 Sep 2022 16:30:50 -0400 Subject: [PATCH 5/6] Event functions can also be referenced within other event functions --- .../__tests__/ESLintRulesOfHooks-test.js | 7 ++++++- packages/eslint-plugin-react-hooks/src/RulesOfHooks.js | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index 996a7873e9b4f..d7b154caa0145 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -441,15 +441,20 @@ const tests = { } `, ` - // Valid because functions created with useEvent can be passed by reference in useEffect. + // 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()} /> } `, ` diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index c9099b567e13b..9d26caf8ac28e 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -572,10 +572,12 @@ export default { // OK - onClick(); resolveUseEventViolation(scope, node.callee); - // useEvent: useEvent functions can be passed by reference within useEffect + // 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' && + (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 From 54765ee34e488988bc143b2523f8bfc8aa1a1aa7 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 23 Sep 2022 17:15:54 -0400 Subject: [PATCH 6/6] Unify experimental check for useEvent --- .../ESLintRuleExhaustiveDeps-test.js | 11 +- .../__tests__/ESLintRulesOfHooks-test.js | 257 +++++++++--------- .../src/ExhaustiveDeps.js | 12 +- .../src/RulesOfHooks.js | 8 +- 4 files changed, 152 insertions(+), 136 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index b6dd25b57024a..2f9764a74db19 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -7631,10 +7631,12 @@ const tests = { }; if (__EXPERIMENTAL__) { - tests.valid.push({ - code: normalizeIndent` + tests.valid = [ + ...tests.valid, + { + code: normalizeIndent` function MyComponent({ theme }) { - const onStuff = experimental_useEvent(() => { + const onStuff = useEvent(() => { showNotification(theme); }); useEffect(() => { @@ -7642,7 +7644,8 @@ if (__EXPERIMENTAL__) { }, []); } `, - }); + }, + ]; } // Tests that are only valid/invalid across parsers supporting Flow diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index d7b154caa0145..16bec9eb588a9 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -406,87 +406,6 @@ const tests = { const [myState, setMyState] = useState(null); } `, - ` - // 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); - }); - } - `, ], invalid: [ { @@ -1052,66 +971,156 @@ const tests = { `, errors: [classError('useState')], }, + ], +}; + +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 ; - } - `, + 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 - } + // 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()} /> - } - `, + // 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 ; - } - `, + 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 - } - `, + // 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 { diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index f496bc8f5e1b5..2ebaf21c62c30 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -225,10 +225,7 @@ export default { if (name === 'useRef' && id.type === 'Identifier') { // useRef() return value is stable. return true; - } else if ( - name === 'experimental_useEvent' && - id.type === 'Identifier' - ) { + } else if (isUseEventIdentifier(callee) && id.type === 'Identifier') { // useEvent() return value is stable. return true; } else if (name === 'useState' || name === 'useReducer') { @@ -1827,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 9d26caf8ac28e..20ceb24244a9e 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -101,10 +101,10 @@ function isInsideComponentOrHook(node) { } function isUseEventIdentifier(node) { - return ( - node.type === 'Identifier' && - (node.name === 'useEvent' || node.name === 'experimental_useEvent') - ); + if (__EXPERIMENTAL__) { + return node.type === 'Identifier' && node.name === 'useEvent'; + } + return false; } export default {