diff --git a/eslint.config.mts b/eslint.config.mts index d610962bd..3a8b0c337 100644 --- a/eslint.config.mts +++ b/eslint.config.mts @@ -201,12 +201,14 @@ const config: FlatConfig[] = [ "dedent", "html", "tsx", + "ts", ], tags: [ "outdent", "dedent", "html", "tsx", + "ts", ], }, ], diff --git a/packages/plugins/eslint-plugin-react-hooks-extra/README.md b/packages/plugins/eslint-plugin-react-hooks-extra/README.md index 261d1aa9d..857989918 100644 --- a/packages/plugins/eslint-plugin-react-hooks-extra/README.md +++ b/packages/plugins/eslint-plugin-react-hooks-extra/README.md @@ -40,9 +40,10 @@ export default [ ## Rules -| Rule | Description | 💼 | 💭 | ❌ | -| :--------------------------------------- | :---------------------------------------------------------------- | :-: | :-: | :-: | -| `ensure-custom-hooks-using-other-hooks` | Warns when custom Hooks that don't use other Hooks. | ✔️ | | | -| `ensure-use-callback-has-non-empty-deps` | Warns when `useCallback` is called with empty dependencies array. | 🧐 | | | -| `ensure-use-memo-has-non-empty-deps` | Warns when `useMemo` is called with empty dependencies array. | 🧐 | | | -| `prefer-use-state-lazy-initialization` | Warns function calls made inside `useState` calls. | 🚀 | | | +| Rule | Description | 💼 | 💭 | ❌ | +| :--------------------------------------- | :------------------------------------------------------------------------ | :-: | :-: | :-: | +| `ensure-custom-hooks-using-other-hooks` | Warns when custom Hooks that don't use other Hooks. | ✔️ | | | +| `ensure-use-callback-has-non-empty-deps` | Warns when `useCallback` is called with empty dependencies array. | 🧐 | | | +| `ensure-use-memo-has-non-empty-deps` | Warns when `useMemo` is called with empty dependencies array. | 🧐 | | | +| `no-direct-set-state-in-use-effect` | Disallow direct calls to the `set` function of `useState` in `useEffect`. | ✔️ | | | +| `prefer-use-state-lazy-initialization` | Warns function calls made inside `useState` calls. | 🚀 | | | diff --git a/packages/plugins/eslint-plugin-react-hooks-extra/src/index.ts b/packages/plugins/eslint-plugin-react-hooks-extra/src/index.ts index 5b1a8daa8..8ebe9a636 100644 --- a/packages/plugins/eslint-plugin-react-hooks-extra/src/index.ts +++ b/packages/plugins/eslint-plugin-react-hooks-extra/src/index.ts @@ -2,6 +2,7 @@ import { name, version } from "../package.json"; import ensureCustomHooksUsingOtherHooks from "./rules/ensure-custom-hooks-using-other-hooks"; import ensureUseCallbackHasNonEmptyDeps from "./rules/ensure-use-callback-has-non-empty-deps"; import ensureUseMemoHasNonEmptyDeps from "./rules/ensure-use-memo-has-non-empty-deps"; +import noDirectSetStateInUseEffect from "./rules/no-direct-set-state-in-use-effect"; import preferUseStateLazyInitialization from "./rules/prefer-use-state-lazy-initialization"; export const meta = { @@ -13,5 +14,6 @@ export const rules = { "ensure-custom-hooks-using-other-hooks": ensureCustomHooksUsingOtherHooks, "ensure-use-callback-has-non-empty-deps": ensureUseCallbackHasNonEmptyDeps, "ensure-use-memo-has-non-empty-deps": ensureUseMemoHasNonEmptyDeps, + "no-direct-set-state-in-use-effect": noDirectSetStateInUseEffect, "prefer-use-state-lazy-initialization": preferUseStateLazyInitialization, } as const; diff --git a/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-effect.spec.ts b/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-effect.spec.ts new file mode 100644 index 000000000..997e43e68 --- /dev/null +++ b/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-effect.spec.ts @@ -0,0 +1,197 @@ +import { allValid, ruleTester } from "../../../../../test"; +import rule, { RULE_NAME } from "./no-direct-set-state-in-use-effect"; + +ruleTester.run(RULE_NAME, rule, { + invalid: [ + { + code: /* tsx */ ` + import { useEffect, useState } from "react"; + + function Component() { + const [data, setData] = useState(0); + useEffect(() => { + setData(1); + }, []); + } + `, + errors: [ + { messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT" }, + ], + }, + { + code: /* tsx */ ` + import { useEffect, useState } from "react"; + + function Component() { + const data = useState(0); + useEffect(() => { + data[1](); + }, []); + } + `, + errors: [ + { messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT" }, + ], + }, + { + code: /* tsx */ ` + import { useEffect, useState } from "react"; + + function Component() { + const data = useState(0); + useEffect(() => { + data.at(1)(); + }, []); + } + `, + errors: [ + { messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT" }, + ], + }, + { + code: /* tsx */ ` + import { useEffect, useState } from "react"; + + const index = 1; + function Component() { + const data = useState(0); + useEffect(() => { + data.at(index)(); + }, []); + } + `, + errors: [ + { messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT" }, + ], + }, + { + code: /* tsx */ ` + import { useEffect, useState } from "react"; + + const index = 1; + function Component() { + const data = useState(0); + useEffect(() => { + data[index](); + }, []); + } + `, + errors: [ + { messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT" }, + ], + }, + { + code: /* tsx */ ` + import { useEffect } from "react"; + + const index = 1; + function Component() { + const data = useCustomState(0); + useEffect(() => { + data[index](); + }, []); + } + `, + errors: [ + { messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT" }, + ], + settings: { + "react-x": { + additionalHooks: { + useState: ["useCustomState"], + }, + }, + }, + }, + { + code: /* tsx */ ` + import { useState } from "react"; + + const index = 1; + function Component() { + const data = useState(0); + useCustomEffect(() => { + data[index](); + }, []); + } + `, + errors: [ + { messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT" }, + ], + settings: { + "react-x": { + additionalHooks: { + useEffect: ["useCustomEffect"], + }, + }, + }, + }, + { + code: /* tsx */ ` + const index = 1; + function Component() { + const data = useCustomState(0); + useCustomEffect(() => { + data[index](); + }, []); + } + `, + errors: [ + { messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT" }, + ], + settings: { + "react-x": { + additionalHooks: { + useEffect: ["useCustomEffect"], + useState: ["useCustomState"], + }, + }, + }, + }, + ], + valid: [ + ...allValid, + /* tsx */ ` + import { useEffect, useState } from "react"; + + function Component() { + const [data, setData] = useState(0); + useEffect(() => { + const handler = () => setData(1); + }, []); + } + `, + /* tsx */ ` + import { useEffect, useState } from "react"; + + function Component() { + const [data, setData] = useState(0); + useEffect(() => { + fetch().then(() => setData()); + }, []); + } + `, + /* tsx */ ` + import { useEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useEffect(() => { + (async () => { setData() })(); + }, []); + } + `, + /* tsx */ ` + import { useEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useEffect(() => { + const onLoad = () => { + setData(); + }; + }, []); + } + `, + ], +}); diff --git a/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-effect.ts b/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-effect.ts new file mode 100644 index 000000000..8905a7dd6 --- /dev/null +++ b/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-effect.ts @@ -0,0 +1,109 @@ +import { is, NodeType, traverseUp } from "@eslint-react/ast"; +import { isReactHookCallWithNameLoose, isUseEffectCall, isUseStateCall } from "@eslint-react/core"; +import { getESLintReactSettings } from "@eslint-react/shared"; +import { F, O } from "@eslint-react/tools"; +import type { RuleContext } from "@eslint-react/types"; +import { findVariable, getVariableInit } from "@eslint-react/var"; +import type { ESLintUtils, TSESTree } from "@typescript-eslint/utils"; +import { getStaticValue } from "@typescript-eslint/utils/ast-utils"; +import type { ConstantCase } from "string-ts"; +import { match } from "ts-pattern"; + +import { createRule } from "../utils"; + +export const RULE_NAME = "no-direct-set-state-in-use-effect"; + +export type MessageID = ConstantCase; + +export default createRule<[], MessageID>({ + meta: { + type: "problem", + docs: { + description: "disallow direct calls to the 'set' function of 'useState' in 'useEffect'.", + }, + messages: { + NO_DIRECT_SET_STATE_IN_USE_EFFECT: "Do not call the set function of 'useState' directly in 'useEffect'.", + }, + schema: [], + }, + name: RULE_NAME, + create(context) { + const settings = getESLintReactSettings(context.settings); + const { useEffect: useEffectAlias = [], useState: useStateAlias = [] } = settings.additionalHooks ?? {}; + function isUseEffectCallWithAlias(node: TSESTree.CallExpression, context: RuleContext) { + return (isUseEffectCall(node, context) || useEffectAlias.some(F.flip(isReactHookCallWithNameLoose)(node))); + } + function isUseStateCallWithAlias(node: TSESTree.CallExpression, context: RuleContext) { + return (isUseStateCall(node, context) || useStateAlias.some(F.flip(isReactHookCallWithNameLoose)(node))); + } + return { + CallExpression(node) { + const effectFunction = traverseUp( + node, + (n) => + n.parent?.type === NodeType.CallExpression + && isUseEffectCallWithAlias(n.parent, context), + ); + // TODO: support detecting effect cleanup functions as well or add a separate rule for that called `no-direct-set-state-in-use-effect-cleanup` + if (O.isNone(effectFunction)) return; + const scope = context.sourceCode.getScope(node); + if (scope.block !== effectFunction.value) return; + const name = match(node.callee) + // const [data, setData] = useState(); + // setData(); + .with({ type: NodeType.Identifier }, (n) => O.some(n.name)) + // const data = useState(); + // data[1](); + .with({ type: NodeType.MemberExpression }, (n) => { + if (!("name" in n.object)) return O.none(); + const initialScope = context.sourceCode.getScope(n); + const property = getStaticValue(n.property, initialScope); + if (property?.value === 1) return O.fromNullable(n.object.name); + return O.none(); + }) + // const data = useState(); + // data.at(1)(); + .with({ type: NodeType.CallExpression }, (n) => { + if (!is(NodeType.MemberExpression)(n.callee)) return O.none(); + if (!("name" in n.callee.object)) return O.none(); + const isAt = match(n.callee) + .with( + { + type: NodeType.MemberExpression, + property: { + type: NodeType.Identifier, + name: "at", + }, + }, + F.constTrue, + ) + .otherwise(F.constFalse); + if (!isAt) return O.none(); + const [index] = n.arguments; + if (!index) return O.none(); + const initialScope = context.sourceCode.getScope(n); + const value = getStaticValue(index, initialScope); + if (value?.value === 1) return O.fromNullable(n.callee.object.name); + return O.none(); + }) + .otherwise(O.none); + F.pipe( + name, + O.flatMap(findVariable(scope)), + O.flatMap(getVariableInit(0)), + O.filter(is(NodeType.CallExpression)), + O.filter(name => isUseStateCallWithAlias(name, context)), + O.map(name => ({ + data: { + setState: name, + }, + messageId: "NO_DIRECT_SET_STATE_IN_USE_EFFECT", + node, + } as const)), + O.map(context.report), + ); + }, + }; + }, + defaultOptions: [], +}) satisfies ESLintUtils.RuleModule; diff --git a/website/pages/docs/glossary.mdx b/website/pages/docs/glossary.mdx index 04f644d6c..2dbc90157 100644 --- a/website/pages/docs/glossary.mdx +++ b/website/pages/docs/glossary.mdx @@ -25,6 +25,10 @@ A React component that lets you group elements without a wrapper node. The shorthand syntax for a Fragment in JSX. It looks like `<>...`. +### Set Function + +The [`set` function](https://react.dev/reference/react/useState#setstate) like `setSomething(nextState)` [returned by `useState`](https://react.dev/reference/react/useState#returns) lets you update the state to a different value and trigger a re-render. + ### Legacy React APIs [APIs](https://react.dev/reference/react/legacy) are exported from the `react` package, but they are not recommended for use in newly written code. diff --git a/website/pages/docs/rules/_meta.ts b/website/pages/docs/rules/_meta.ts index f746dd173..da4bcba38 100644 --- a/website/pages/docs/rules/_meta.ts +++ b/website/pages/docs/rules/_meta.ts @@ -73,6 +73,7 @@ export default { "hooks-extra-ensure-custom-hooks-using-other-hooks": "hooks-extra/ensure-custom-hooks-using-other-hooks", "hooks-extra-ensure-use-callback-has-non-empty-deps": "hooks-extra/ensure-use-callback-has-non-empty-deps", "hooks-extra-ensure-use-memo-has-non-empty-deps": "hooks-extra/ensure-use-memo-has-non-empty-deps", + "hooks-extra-no-direct-set-state-in-use-effect": "hooks-extra/no-direct-set-state-in-use-effect", "hooks-extra-prefer-use-state-lazy-initialization": "hooks-extra/prefer-use-state-lazy-initialization", "naming-convention-component-name": "naming-convention/component-name", "naming-convention-filename": "naming-convention/filename", diff --git a/website/pages/docs/rules/hooks-extra-no-set-state-in-use-effect.mdx b/website/pages/docs/rules/hooks-extra-no-set-state-in-use-effect.mdx new file mode 100644 index 000000000..bde8fc6ed --- /dev/null +++ b/website/pages/docs/rules/hooks-extra-no-set-state-in-use-effect.mdx @@ -0,0 +1,58 @@ +# no-direct-set-state-in-use-effect + +## Rule category + +Correctness. + +## What it does + +Disallow direct calls to the [`set` function](https://react.dev/reference/react/useState#setstate) of `useState` in `useEffect`. + +## Why is this bad? + +Calling `setState` directly in `useEffect` is a common bad practice. It can lead to infinite update loops and other side effects. + +## Examples + +### Failing + +```tsx twoslash +import React, { useEffect, useState } from "react"; + +function Example() { + const [value, setValue] = useState(0); + + useEffect(() => { + setValue(value + 1); + // ^^^^^^^^^^^^^^^^^ + // - Do not call the 'set' function of 'useState' directly in 'useEffect'. + }, []); + + return null; +} +``` + +### Passing + +```tsx twoslash +import React, { useEffect, useState } from "react"; + +function Example() { + const [value, setValue] = useState(0); + + useEffect(() => { + const onLoad = () => { + setValue(value + 1); + }; + // ... + }, []); + + return null; +} +``` + +### Further Reading + +- [React: useState](https://react.dev/reference/react/useState#setstate) +- [React: useEffect](https://react.dev/reference/react/useEffect) +- [React: You Might Not Need an Effect](https://react.dev/learn/you-might-not-need-an-effect) diff --git a/website/pages/docs/rules/overview.md b/website/pages/docs/rules/overview.md index cd65f48b2..1fbfa3f91 100644 --- a/website/pages/docs/rules/overview.md +++ b/website/pages/docs/rules/overview.md @@ -82,12 +82,13 @@ ## Hooks Extra Rules -| Rule | Description | 💼 | 💭 | ❌ | -| :--------------------------------------------------------------------------------------------------------- | :---------------------------------------------------------------- | :-: | :-: | :-: | -| [`hooks-extra/ensure-custom-hooks-using-other-hooks`](hooks-extra-ensure-custom-hooks-using-other-hooks) | Warns when custom Hooks that don't use other Hooks. | ✔️ | | | -| [`hooks-extra/ensure-use-callback-has-non-empty-deps`](hooks-extra-ensure-use-callback-has-non-empty-deps) | Warns when `useCallback` is called with empty dependencies array. | 🧐 | | | -| [`hooks-extra/ensure-use-memo-has-non-empty-deps`](hooks-extra-ensure-use-memo-has-non-empty-deps) | Warns when `useMemo` is called with empty dependencies array. | 🧐 | | | -| [`hooks-extra/prefer-use-state-lazy-initialization`](hooks-extra-prefer-use-state-lazy-initialization) | Warns function calls made inside `useState` calls. | 🚀 | | | +| Rule | Description | 💼 | 💭 | ❌ | +| :--------------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------ | :-: | :-: | :-: | +| [`hooks-extra/ensure-custom-hooks-using-other-hooks`](hooks-extra-ensure-custom-hooks-using-other-hooks) | Warns when custom Hooks that don't use other Hooks. | ✔️ | | | +| [`hooks-extra/ensure-use-callback-has-non-empty-deps`](hooks-extra-ensure-use-callback-has-non-empty-deps) | Warns when `useCallback` is called with empty dependencies array. | 🧐 | | | +| [`hooks-extra/ensure-use-memo-has-non-empty-deps`](hooks-extra-ensure-use-memo-has-non-empty-deps) | Warns when `useMemo` is called with empty dependencies array. | 🧐 | | | +| [`hooks-extra/no-direct-set-state-in-use-effect`](hooks-extra-no-direct-set-state-in-use-effect) | Disallow direct calls to the `set` function of `useState` in `useEffect`. | ✔️ | | | +| [`hooks-extra/prefer-use-state-lazy-initialization`](hooks-extra-prefer-use-state-lazy-initialization) | Warns function calls made inside `useState` calls. | 🚀 | | | ## Naming Convention Rules diff --git a/website/pages/roadmap.md b/website/pages/roadmap.md index 346444e95..feaa8a3d8 100644 --- a/website/pages/roadmap.md +++ b/website/pages/roadmap.md @@ -75,10 +75,12 @@ ### Rules in `eslint-plugin-react-hooks-extra` -| Rule | Description | -| :-------------------------------------- | :-------------------------------------------------- | -| `ensure-custom-hooks-using-other-hooks` | Warns when custom Hooks that don't use other Hooks. | -| `prefer-use-state-lazy-initialization` | Warns function calls made inside `useState` calls. | +| Rule | Description | +| :----------------------------------------- | :-------------------------------------------------------------------------- | +| `ensure-custom-hooks-using-other-hooks` | Warns when custom Hooks that don't use other Hooks. | +| `no-direct-set-state-in-use-effect` | Disallow direct calls to `set` function of `useState` in `useEffect`. | +| `no-direct-set-state-in-use-layout-effect` | Disallow direct calls to `set` function of `useState` in `useLayoutEffect`. | +| `prefer-use-state-lazy-initialization` | Warns function calls made inside `useState` calls. | ### Rules in `eslint-plugin-react-naming-convention`