[eslint] Do not allow useEffectEvent fns to be called in arbitrary closures (facebook#33544)

Summary:

useEffectEvent is meant to be used specifically in combination with
useEffect, and using
the feature in arbitrary closures can lead to surprising reactivity
semantics. In order to
minimize risk in the experimental rollout, we are going to restrict its
usage to being
called directly inside an effect or another useEffectEvent, effectively
enforcing the function
coloring statically. Without an effect system this is the best we can
do.

Author: jbrown215

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

@@ -1343,33 +1343,6 @@ if (__EXPERIMENTAL__) {
         }
       `,
     },
-    {
-      code: normalizeIndent`
-        // Valid because functions created with useEffectEvent can be called in closures.
-        function MyComponent({ theme }) {
-          const onClick = useEffectEvent(() => {
-            showNotification(theme);
-          });
-          return <Child onClick={() => onClick()}></Child>;
-        }
-      `,
-    },
-    {
-      code: normalizeIndent`
-        // Valid because functions created with useEffectEvent can be called in closures.
-        function MyComponent({ theme }) {
-          const onClick = useEffectEvent(() => {
-            showNotification(theme);
-          });
-          const onClick2 = () => { onClick() };
-          const onClick3 = useCallback(() => onClick(), []);
-          return <>
-            <Child onClick={onClick2}></Child>
-            <Child onClick={onClick3}></Child>
-          </>;
-        }
-      `,
-    },
     {
       code: normalizeIndent`
         // Valid because functions created with useEffectEvent can be passed by reference in useEffect
@@ -1380,47 +1353,39 @@ if (__EXPERIMENTAL__) {
           });
           const onClick2 = useEffectEvent(() => {
             debounce(onClick);
+            debounce(() => onClick());
+            debounce(() => { onClick() });
+            deboucne(() => debounce(onClick));
           });
           useEffect(() => {
-            let id = setInterval(onClick, 100);
+            let id = setInterval(() => onClick(), 100);
             return () => clearInterval(onClick);
           }, []);
-          return <Child onClick={() => onClick2()} />
+          return null;
         }
       `,
     },
-    {
-      code: normalizeIndent`
-        const MyComponent = ({theme}) => {
-          const onClick = useEffectEvent(() => {
-            showNotification(theme);
-          });
-          return <Child onClick={() => onClick()}></Child>;
-        };
-      `,
-    },
     {
       code: normalizeIndent`
         function MyComponent({ theme }) {
-          const notificationService = useNotifications();
-          const showNotification = useEffectEvent((text) => {
-            notificationService.notify(theme, text);
+          useEffect(() => {
+            onClick();
           });
-          const onClick = useEffectEvent((text) => {
-            showNotification(text);
+          const onClick = useEffectEvent(() => {
+            showNotification(theme);
           });
-          return <Child onClick={(text) => onClick(text)} />
         }
       `,
     },
     {
       code: normalizeIndent`
         function MyComponent({ theme }) {
-          useEffect(() => {
-            onClick();
+          const onEvent = useEffectEvent((text) => {
+            console.log(text);
           });
-          const onClick = useEffectEvent(() => {
-            showNotification(theme);
+          
+          useEffect(() => {
+            onEvent('Hello world');
           });
         }
       `,
@@ -1437,7 +1402,7 @@ if (__EXPERIMENTAL__) {
           return <Child onClick={onClick}></Child>;
         }
       `,
-      errors: [useEffectEventError('onClick')],
+      errors: [useEffectEventError('onClick', false)],
     },
     {
       code: normalizeIndent`
@@ -1456,8 +1421,23 @@ if (__EXPERIMENTAL__) {
           });
           return <Child onClick={() => onClick()} />
         }
+          
+        // The useEffectEvent function shares an identifier name with the above
+        function MyLastComponent({theme}) {
+          const onClick = useEffectEvent(() => {
+            showNotification(theme)
+          });
+          useEffect(() => {
+            onClick(); // No error here, errors on all other uses
+            onClick;
+          })
+          return <Child />
+        }
       `,
-      errors: [{...useEffectEventError('onClick'), line: 7}],
+      errors: [
+        {...useEffectEventError('onClick', false), line: 7},
+        {...useEffectEventError('onClick', true), line: 15},
+      ],
     },
     {
       code: normalizeIndent`
@@ -1468,7 +1448,7 @@ if (__EXPERIMENTAL__) {
           return <Child onClick={onClick}></Child>;
         }
       `,
-      errors: [useEffectEventError('onClick')],
+      errors: [useEffectEventError('onClick', false)],
     },
     {
       code: normalizeIndent`
@@ -1481,7 +1461,7 @@ if (__EXPERIMENTAL__) {
           return <Bar onClick={foo} />
         }
       `,
-      errors: [{...useEffectEventError('onClick'), line: 7}],
+      errors: [{...useEffectEventError('onClick', false), line: 7}],
     },
     {
       code: normalizeIndent`
@@ -1497,7 +1477,27 @@ if (__EXPERIMENTAL__) {
           return <Child onClick={onClick} />
         }
       `,
-      errors: [useEffectEventError('onClick')],
+      errors: [useEffectEventError('onClick', false)],
+    },
+    {
+      code: normalizeIndent`
+        // Invalid because functions created with useEffectEvent cannot be called in arbitrary closures.
+        function MyComponent({ theme }) {
+          const onClick = useEffectEvent(() => {
+            showNotification(theme);
+          });
+          const onClick2 = () => { onClick() };
+          const onClick3 = useCallback(() => onClick(), []);
+          return <>
+            <Child onClick={onClick2}></Child>
+            <Child onClick={onClick3}></Child>
+          </>;
+        }
+      `,
+      errors: [
+        useEffectEventError('onClick', true),
+        useEffectEventError('onClick', true),
+      ],
     },
   ];
 }
@@ -1559,11 +1559,11 @@ function classError(hook) {
   };
 }
 
-function useEffectEventError(fn) {
+function useEffectEventError(fn, called) {
   return {
     message:
       `\`${fn}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +
-      'the same component. They cannot be assigned to variables or passed down.',
+      `the same component.${called ? '' : ' They cannot be assigned to variables or passed down.'}`,
   };
 }
 

packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts

@@ -541,7 +541,9 @@ const rule = {
               context.report({
                 node: hook,
                 message:
-                  `React Hook "${getSourceCode().getText(hook)}" may be executed ` +
+                  `React Hook "${getSourceCode().getText(
+                    hook,
+                  )}" may be executed ` +
                   'more than once. Possibly because it is called in a loop. ' +
                   'React Hooks must be called in the exact same order in ' +
                   'every component render.',
@@ -596,7 +598,9 @@ const rule = {
             ) {
               // Custom message for hooks inside a class
               const message =
-                `React Hook "${getSourceCode().getText(hook)}" cannot be called ` +
+                `React Hook "${getSourceCode().getText(
+                  hook,
+                )}" cannot be called ` +
                 'in a class component. React Hooks must be called in a ' +
                 'React function component or a custom React Hook function.';
               context.report({node: hook, message});
@@ -613,7 +617,9 @@ const rule = {
             } else if (codePathNode.type === 'Program') {
               // These are dangerous if you have inline requires enabled.
               const message =
-                `React Hook "${getSourceCode().getText(hook)}" cannot be called ` +
+                `React Hook "${getSourceCode().getText(
+                  hook,
+                )}" cannot be called ` +
                 'at the top level. React Hooks must be called in a ' +
                 'React function component or a custom React Hook function.';
               context.report({node: hook, message});
@@ -626,7 +632,9 @@ const rule = {
               // `use(...)` can be called in callbacks.
               if (isSomewhereInsideComponentOrHook && !isUseIdentifier(hook)) {
                 const message =
-                  `React Hook "${getSourceCode().getText(hook)}" cannot be called ` +
+                  `React Hook "${getSourceCode().getText(
+                    hook,
+                  )}" cannot be called ` +
                   'inside a callback. React Hooks must be called in a ' +
                   'React function component or a custom React Hook function.';
                 context.report({node: hook, message});
@@ -681,18 +689,18 @@ const rule = {
       Identifier(node) {
         // This identifier resolves to a useEffectEvent function, but isn't being referenced in an
         // effect or another event function. It isn't being called either.
-        if (
-          lastEffect == null &&
-          useEffectEventFunctions.has(node) &&
-          node.parent.type !== 'CallExpression'
-        ) {
+        if (lastEffect == null && useEffectEventFunctions.has(node)) {
+          const message =
+            `\`${getSourceCode().getText(
+              node,
+            )}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +
+            'the same component.' +
+            (node.parent.type === 'CallExpression'
+              ? ''
+              : ' They cannot be assigned to variables or passed down.');
           context.report({
             node,
-            message:
-              `\`${getSourceCode().getText(
-                node,
-              )}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +
-              'the same component. They cannot be assigned to variables or passed down.',
+            message,
           });
         }
       },