Handle useEffectEvent - should never be included in effect deps

Author: jackpope

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts

@@ -25,6 +25,7 @@ import {
   Identifier,
   IdentifierId,
   InstructionKind,
+  isEffectEventFunctionType,
   isPrimitiveType,
   isStableType,
   isSubPath,
@@ -317,6 +318,12 @@ function validateDependencies(
       reason: 'Unexpected function dependency',
       loc: inferredDependency.loc,
     });
+    /**
+     * Skip effect event functions as they are not valid dependencies
+     */
+    if (isEffectEventFunctionType(inferredDependency.identifier)) {
+      continue;
+    }
     let hasMatchingManualDependency = false;
     for (const manualDependency of manualDependencies) {
       if (
@@ -339,6 +346,7 @@ function validateDependencies(
     ) {
       continue;
     }
+
     missing.push(inferredDependency);
   }
 
@@ -416,6 +424,17 @@ function validateDependencies(
           },
         );
         if (
+          matchingInferred != null &&
+          isEffectEventFunctionType(matchingInferred.identifier)
+        ) {
+          diagnostic.withDetails({
+            kind: 'error',
+            message:
+              `Functions returned from \`useEffectEvent\` must not be included in the dependency array. ` +
+              `Remove \`${printManualMemoDependency(dep)}\` from the dependencies.`,
+            loc: dep.loc ?? manualMemoLoc,
+          });
+        } else if (
           matchingInferred != null &&
           !isOptionalDependency(matchingInferred, reactive)
         ) {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md

@@ -0,0 +1,80 @@
+
+## Input
+
+```javascript
+// @validateExhaustiveEffectDependencies
+import {useEffect, useEffectEvent} from 'react';
+
+function Component({x, y, z}) {
+  const effectEvent = useEffectEvent(() => {
+    log(x);
+  });
+
+  const effectEvent2 = useEffectEvent(z => {
+    log(y, z);
+  });
+
+  // error - do not include effect event in deps
+  useEffect(() => {
+    effectEvent();
+  }, [effectEvent]);
+
+  // error - do not include effect event in deps
+  useEffect(() => {
+    effectEvent2(z);
+  }, [effectEvent2, z]);
+
+  // error - do not include effect event captured values in deps
+  useEffect(() => {
+    effectEvent2(z);
+  }, [y, z]);
+}
+
+```
+
+
+## Error
+
+```
+Found 3 errors:
+
+Error: Found extra effect dependencies
+
+Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects.
+
+error.exhaustive-deps-effect-events.ts:16:6
+  14 |   useEffect(() => {
+  15 |     effectEvent();
+> 16 |   }, [effectEvent]);
+     |       ^^^^^^^^^^^ Functions returned from `useEffectEvent` must not be included in the dependency array. Remove `effectEvent` from the dependencies.
+  17 |
+  18 |   // error - do not include effect event in deps
+  19 |   useEffect(() => {
+
+Error: Found extra effect dependencies
+
+Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects.
+
+error.exhaustive-deps-effect-events.ts:21:6
+  19 |   useEffect(() => {
+  20 |     effectEvent2(z);
+> 21 |   }, [effectEvent2, z]);
+     |       ^^^^^^^^^^^^ Functions returned from `useEffectEvent` must not be included in the dependency array. Remove `effectEvent2` from the dependencies.
+  22 |
+  23 |   // error - do not include effect event captured values in deps
+  24 |   useEffect(() => {
+
+Error: Found extra effect dependencies
+
+Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects.
+
+error.exhaustive-deps-effect-events.ts:26:6
+  24 |   useEffect(() => {
+  25 |     effectEvent2(z);
+> 26 |   }, [y, z]);
+     |       ^ Unnecessary dependency `y`
+  27 | }
+  28 |
+```
+          
+      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.js

@@ -0,0 +1,27 @@
+// @validateExhaustiveEffectDependencies
+import {useEffect, useEffectEvent} from 'react';
+
+function Component({x, y, z}) {
+  const effectEvent = useEffectEvent(() => {
+    log(x);
+  });
+
+  const effectEvent2 = useEffectEvent(z => {
+    log(y, z);
+  });
+
+  // error - do not include effect event in deps
+  useEffect(() => {
+    effectEvent();
+  }, [effectEvent]);
+
+  // error - do not include effect event in deps
+  useEffect(() => {
+    effectEvent2(z);
+  }, [effectEvent2, z]);
+
+  // error - do not include effect event captured values in deps
+  useEffect(() => {
+    effectEvent2(z);
+  }, [y, z]);
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md

@@ -2,7 +2,7 @@
 ## Input
 
 ```javascript
-// @validateExhaustiveMemoizationDependencies
+// @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies
 import {
   useCallback,
   useTransition,
@@ -21,6 +21,24 @@ function useFoo() {
   const [v, dispatch] = useReducer(() => {}, null);
   const [isPending, dispatchAction] = useActionState(() => {}, null);
 
+  useEffect(() => {
+    dispatch();
+    startTransition(() => {});
+    addOptimistic();
+    setState(null);
+    dispatchAction();
+    ref.current = true;
+  }, [
+    // intentionally adding unnecessary deps on nonreactive stable values
+    // to check that they're allowed
+    dispatch,
+    startTransition,
+    addOptimistic,
+    setState,
+    dispatchAction,
+    ref,
+  ]);
+
   return useCallback(() => {
     dispatch();
     startTransition(() => {});
@@ -50,7 +68,7 @@ export const FIXTURE_ENTRYPOINT = {
 ## Code
 
 ```javascript
-import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies
+import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies
 import {
   useCallback,
   useTransition,
@@ -62,14 +80,15 @@ import {
 } from "react";
 
 function useFoo() {
-  const $ = _c(1);
+  const $ = _c(3);
   const [, setState] = useState();
   const ref = useRef(null);
   const [, startTransition] = useTransition();
   const [, addOptimistic] = useOptimistic();
   const [, dispatch] = useReducer(_temp, null);
   const [, dispatchAction] = useActionState(_temp2, null);
   let t0;
+  let t1;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
     t0 = () => {
       dispatch();
@@ -79,12 +98,38 @@ function useFoo() {
       dispatchAction();
       ref.current = true;
     };
+    t1 = [
+      dispatch,
+      startTransition,
+      addOptimistic,
+      setState,
+      dispatchAction,
+      ref,
+    ];
     $[0] = t0;
+    $[1] = t1;
   } else {
     t0 = $[0];
+    t1 = $[1];
+  }
+  useEffect(t0, t1);
+  let t2;
+  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+    t2 = () => {
+      dispatch();
+      startTransition(_temp4);
+      addOptimistic();
+      setState(null);
+      dispatchAction();
+      ref.current = true;
+    };
+    $[2] = t2;
+  } else {
+    t2 = $[2];
   }
-  return t0;
+  return t2;
 }
+function _temp4() {}
 function _temp3() {}
 function _temp2() {}
 function _temp() {}
@@ -97,4 +142,4 @@ export const FIXTURE_ENTRYPOINT = {
 ```
       
 ### Eval output
-(kind: ok) "[[ function params=0 ]]"
+(kind: exception) useEffect is not defined

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js

@@ -1,4 +1,4 @@
-// @validateExhaustiveMemoizationDependencies
+// @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies
 import {
   useCallback,
   useTransition,
@@ -17,6 +17,24 @@ function useFoo() {
   const [v, dispatch] = useReducer(() => {}, null);
   const [isPending, dispatchAction] = useActionState(() => {}, null);
 
+  useEffect(() => {
+    dispatch();
+    startTransition(() => {});
+    addOptimistic();
+    setState(null);
+    dispatchAction();
+    ref.current = true;
+  }, [
+    // intentionally adding unnecessary deps on nonreactive stable values
+    // to check that they're allowed
+    dispatch,
+    startTransition,
+    addOptimistic,
+    setState,
+    dispatchAction,
+    ref,
+  ]);
+
   return useCallback(() => {
     dispatch();
     startTransition(() => {});