diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.mdx b/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.mdx index 94667289b..457055db0 100644 --- a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.mdx +++ b/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
Hello World
; +} +``` + ### 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
Hello World
; +} +``` + ## Implementation - [Rule Source](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.ts) diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.spec.ts b/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.spec.ts index 959a4e2db..918e39414 100644 --- a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.spec.ts +++ b/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
items
; + } + `, + 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
items
; + } + `, + 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
items
; + } + + function App({ items }) { + const [test, setTest] = useState(0); + + const updateTest = useCallback(() => {setTest(items.length)}, [items]); + + useEffect(() => { + updateTest(); + }, [updateTest]); + + return
items
; + } + `, + 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
items
; + } + `, + 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
updateTest()}>{test}
; + } + `, + 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
; + }; + `, + 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
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
; + }; + `, ], }); diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.ts b/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.ts index a080f1e95..7ec3cf9c4 100644 --- a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-callback.ts +++ b/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; +export type MessageID = CamelCase | "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): 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): 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): RuleListener { messageId: "noUnnecessaryUseCallback", node, }); + return; } + report(context)(checkForUsageInsideUseEffectReport); }, }; } + +function checkForUsageInsideUseEffect( + sourceCode: Readonly, + node: TSESTree.CallExpression, +): ReportDescriptor | 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(); + + 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 }, + }; +} diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-memo.mdx b/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-memo.mdx index 9a164deb1..69dda8bdc 100644 --- a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-memo.mdx +++ b/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-use-memo.mdx @@ -1,81 +1,112 @@ ---- -title: no-unnecessary-use-memo ---- - -**Full Name in `@eslint-react/eslint-plugin`** - -```plain copy -@eslint-react/no-unnecessary-use-memo -``` - -**Full Name in `eslint-plugin-react-x`** - -```plain copy -react-x/no-unnecessary-use-memo -``` - -**Features** - -`🧪` - -**Presets** - -`strict` -`strict-typescript` -`strict-type-checked` - -## Description - -Disallow unnecessary usage of `useMemo`. - -React Hooks `useMemo` has empty dependencies array like what's in the examples, are unnecessary. The hook can be removed and it's value can be calculated in the component body or hoisted to the outer scope of the component. - -## Examples - -### Failing - -```tsx -import { Button, MantineTheme } from "@mantine/core"; -import React, { useMemo } from "react"; - -function MyComponent() { - const style = useMemo( - (theme: MantineTheme) => ({ - input: { - fontFamily: theme.fontFamilyMonospace, - }, - }), - [], - ); - return