feat: Disallow a function wrapped in useCallback or a variable wrapped in useMemo yet only used in useEffect and only in one useEffect, closes #1278 (#1321)

Co-authored-by: REL1CX <[email protected]>

Author: possum-enjoyer

packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.mdx

@@ -29,6 +29,7 @@ react-x/no-unnecessary-use-callback
 Disallow unnecessary usage of `useCallback`.
 
 React Hooks `useCallback` has empty dependencies array like what's in the examples, are unnecessary. The hook can be removed and it's value can be created in the component body or hoisted to the outer scope of the component.
+If the calculated function is only used inside one useEffect the calculation can be moved inside the useEffect Function.
 
 ## Examples
 
@@ -46,6 +47,22 @@ function MyComponent() {
 }
 ```
 
+
+```tsx
+import { Button, MantineTheme } from "@mantine/core";
+import React, { useCallback, useEffect } from "react";
+
+function MyComponent({items}: {items: string[]}) {
+  const updateTest = useCallback(() => {console.log(items.length)}, [items]);
+
+  useEffect(() => {
+    updateTest();
+  }, [updateTest]);
+
+  return <div>Hello World</div>;
+}
+```
+
 ### Passing
 
 ```tsx
@@ -60,6 +77,19 @@ function MyComponent() {
 }
 ```
 
+```tsx
+import { Button, MantineTheme } from "@mantine/core";
+import React, { useEffect } from "react";
+
+function MyComponent({items}: {items: string[]}) {
+  useEffect(() => {
+    console.log(items.length);
+  }, [items]);
+
+  return <div>Hello World</div>;
+}
+```
+
 ## Implementation
 
 - [Rule Source](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.ts)

packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.spec.ts

@@ -405,6 +405,130 @@ ruleTester.run(RULE_NAME, rule, {
         },
       },
     },
+
+    {
+      code: tsx`
+        import {useCallback, useState, useEffect} from 'react';
+
+        function App({ items }) {
+          const [test, setTest] = useState(0);
+
+          const updateTest = useCallback(() => {setTest(items.length)}, [items]);
+
+          useEffect(() => {
+            updateTest();
+          }, [updateTest]);
+
+          return <div>items</div>;
+        }
+      `,
+      errors: [
+        {
+          messageId: "noUnnecessaryUseCallbackInsideUseEffect",
+        },
+      ],
+      settings: {
+        "react-x": {
+          importSource: "react",
+        },
+      },
+    },
+    {
+      code: tsx`
+        import {useCallback, useState, useEffect} from 'react';
+
+        function App({ items }) {
+          const [test, setTest] = useState(0);
+
+          const updateTest = useCallback(() => {console.log('test')}, []);
+
+          useEffect(() => {
+            updateTest();
+          }, [updateTest]);
+
+          return <div>items</div>;
+        }
+      `,
+      errors: [
+        {
+          messageId: "noUnnecessaryUseCallback",
+        },
+      ],
+      settings: {
+        "react-x": {
+          importSource: "react",
+        },
+      },
+    },
+    {
+      code: tsx`
+        import {useCallback, useState, useEffect} from 'react';
+
+        function App({ items }) {
+          const [test, setTest] = useState(0);
+
+          const updateTest = useCallback(() => {setTest(items.length)}, [items]);
+
+          useEffect(() => {
+            updateTest();
+          }, [updateTest]);
+
+          return <div>items</div>;
+        }
+
+          function App({ items }) {
+          const [test, setTest] = useState(0);
+
+          const updateTest = useCallback(() => {setTest(items.length)}, [items]);
+
+          useEffect(() => {
+            updateTest();
+          }, [updateTest]);
+
+          return <div>items</div>;
+        }
+      `,
+      errors: [
+        {
+          messageId: "noUnnecessaryUseCallbackInsideUseEffect",
+        },
+        {
+          messageId: "noUnnecessaryUseCallbackInsideUseEffect",
+        },
+      ],
+      settings: {
+        "react-x": {
+          importSource: "react",
+        },
+      },
+    },
+    {
+      code: tsx`
+        const { useCallback, useEffect } = require("@pika/react");
+
+        function App({ items }) {
+          const [test, setTest] = useState(0);
+
+          const updateTest = useCallback(() => {setTest(items.length)}, [items]);
+
+          useEffect(() => {
+            updateTest();
+          }, [updateTest]);
+
+          return <div>items</div>;
+        }
+      `,
+      errors: [
+        {
+          messageId: "noUnnecessaryUseCallbackInsideUseEffect",
+        },
+      ],
+      settings: {
+        "react-x": {
+          importSource: "@pika/react",
+        },
+      },
+    },
   ],
   valid: [
     ...allValid,
@@ -501,5 +625,70 @@ ruleTester.run(RULE_NAME, rule, {
               const refItem = useCallback(cb, deps)
       };
     `,
+
+    tsx`
+      import { useCallback, useState, useEffect } from 'react';
+
+      function App({ items }) {
+        const [test, setTest] = useState(items.length);
+
+        const updateTest = useCallback(() => { setTest(items.length + 1) }, [setTest, items]);
+
+        useEffect(function () {
+          function foo() {
+            updateTest();
+          }
+
+          foo();
+
+          updateTest();
+        }, [updateTest])
+
+        return <div onClick={() => updateTest()}>{test}</div>;
+      }
+    `,
+    tsx`
+      import { useCallback, useState, useEffect } from 'react';
+
+      const Component = () => {
+          const [test, setTest] = useState(items.length);
+
+          const updateTest = useCallback(() => { setTest(items.length + 1) }, [setTest, items]);
+
+          useEffect(() => {
+            // some condition
+            updateTest();
+          }, [updateTest]);
+
+          useEffect(() => {
+            // some condition
+            updateTest();
+          }, [updateTest]);
+
+          return <div />;
+      };
+    `,
+    tsx`
+      import { useCallback, useState, useEffect } from 'react';
+
+      const Component = () => {
+        const [test, setTest] = useState(items.length);
+
+        const updateTest = useCallback(() => { setTest(items.length + 1) }, [setTest, items]);
+
+        return <div ref={() => updateTest()} />;
+      };
+    `,
+    tsx`
+      import { useCallback, useState, useEffect } from 'react';
+
+      const Component = () => {
+        const [test, setTest] = useState(items.length);
+
+        const updateTest = useCallback(() => { setTest(items.length + 1) }, [setTest, items]);
+
+        return <div onClick={updateTest} />;
+      };
+    `,
   ],
 });

packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.ts

@@ -1,13 +1,14 @@
 import * as AST from "@eslint-react/ast";
-import { isUseCallbackCall } from "@eslint-react/core";
+import { isUseCallbackCall, isUseEffectLikeCall } from "@eslint-react/core";
 import { identity } from "@eslint-react/eff";
-import type { RuleContext, RuleFeature } from "@eslint-react/shared";
+import { type RuleContext, type RuleFeature, report } from "@eslint-react/shared";
 import { findVariable, getChildScopes, getVariableDefinitionNode } from "@eslint-react/var";
+import type { TSESTree } from "@typescript-eslint/types";
 import { AST_NODE_TYPES as T } from "@typescript-eslint/types";
-import type { RuleListener } from "@typescript-eslint/utils/ts-eslint";
+import { isIdentifier, isVariableDeclarator } from "@typescript-eslint/utils/ast-utils";
+import { type ReportDescriptor, type RuleListener, type SourceCode } from "@typescript-eslint/utils/ts-eslint";
 import type { CamelCase } from "string-ts";
 import { match } from "ts-pattern";
-
 import { createRule } from "../utils";
 
 export const RULE_NAME = "no-unnecessary-use-callback";
@@ -16,7 +17,7 @@ export const RULE_FEATURES = [
   "EXP",
 ] as const satisfies RuleFeature[];
 
-export type MessageID = CamelCase<typeof RULE_NAME>;
+export type MessageID = CamelCase<typeof RULE_NAME> | "noUnnecessaryUseCallbackInsideUseEffect";
 
 export default createRule<[], MessageID>({
   meta: {
@@ -28,6 +29,8 @@ export default createRule<[], MessageID>({
     messages: {
       noUnnecessaryUseCallback:
         "An 'useCallback' with empty deps and no references to the component scope may be unnecessary.",
+      noUnnecessaryUseCallbackInsideUseEffect:
+        "{{name}} is only used inside 1 useEffect, which may be unnecessary. You can move the computation into useEffect directly and merge the dependency arrays.",
     },
     schema: [],
   },
@@ -39,13 +42,18 @@ export default createRule<[], MessageID>({
 export function create(context: RuleContext<MessageID, []>): RuleListener {
   // Fast path: skip if `useCallback` is not present in the file
   if (!context.sourceCode.text.includes("useCallback")) return {};
+
   return {
     CallExpression(node) {
       if (!isUseCallbackCall(node)) {
         return;
       }
+
+      const checkForUsageInsideUseEffectReport = checkForUsageInsideUseEffect(context.sourceCode, node);
+
       const initialScope = context.sourceCode.getScope(node);
       const component = context.sourceCode.getScope(node).block;
+
       if (!AST.isFunction(component)) {
         return;
       }
@@ -67,8 +75,10 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
         .otherwise(() => false);
 
       if (!hasEmptyDeps) {
+        report(context)(checkForUsageInsideUseEffectReport);
         return;
       }
+
       const arg0Node = match(arg0)
         .with({ type: T.ArrowFunctionExpression }, (n) => {
           if (n.body.type === T.ArrowFunctionExpression) {
@@ -97,7 +107,46 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
           messageId: "noUnnecessaryUseCallback",
           node,
         });
+        return;
       }
+      report(context)(checkForUsageInsideUseEffectReport);
     },
   };
 }
+
+function checkForUsageInsideUseEffect(
+  sourceCode: Readonly<SourceCode>,
+  node: TSESTree.CallExpression,
+): ReportDescriptor<MessageID> | undefined {
+  if (!/use\w*Effect/u.test(sourceCode.text)) return;
+
+  if (!isVariableDeclarator(node.parent)) {
+    return;
+  }
+
+  if (!isIdentifier(node.parent.id)) {
+    return;
+  }
+
+  const references = sourceCode.getDeclaredVariables(node.parent)[0]?.references ?? [];
+  const usages = references.filter((ref) => !(ref.init ?? false));
+  const effectSet = new Set<TSESTree.Node>();
+
+  for (const usage of usages) {
+    const effect = AST.findParentNode(usage.identifier, isUseEffectLikeCall);
+
+    if (effect == null) {
+      return;
+    }
+
+    effectSet.add(effect);
+    if (effectSet.size > 1) {
+      return;
+    }
+  }
+  return {
+    messageId: "noUnnecessaryUseCallbackInsideUseEffect",
+    node,
+    data: { name: node.parent.id.name },
+  };
+}