From 185f3ddf03afe0b686ae86e41f037efe7975c188 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Fri, 23 Sep 2022 17:15:54 -0400 Subject: [PATCH] Unify experimental check for useEvent --- .../ESLintRuleExhaustiveDeps-test.js | 9 +- .../__tests__/ESLintRulesOfHooks-test.js | 257 +++++++++--------- .../src/ExhaustiveDeps.js | 12 +- .../src/RulesOfHooks.js | 8 +- 4 files changed, 151 insertions(+), 135 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..50936dee6a8b5 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -7631,8 +7631,10 @@ const tests = { }; if (__EXPERIMENTAL__) { - tests.valid.push({ - code: normalizeIndent` + tests.valid = [ + ...tests.valid, + { + code: normalizeIndent` function MyComponent({ theme }) { const onStuff = experimental_useEvent(() => { showNotification(theme); @@ -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 {