From 6bda4bf79f63e6ebf0230636eadd768c051a1924 Mon Sep 17 00:00:00 2001 From: Eva1ent <rel1cx@proton.me> Date: Thu, 10 Oct 2024 23:03:12 +0800 Subject: [PATCH] refactor: Undeprecate `hooks-extra-no-direct-set-state-in-use-layout-effect` and remove it from recommended presets, closes #839 (#840) --- .../src/index.ts | 4 +- .../no-direct-set-state-in-use-effect.spec.ts | 70 +- .../no-direct-set-state-in-use-effect.ts | 12 +- ...ect-set-state-in-use-layout-effect.spec.ts | 812 ++++++++++++++++++ ...o-direct-set-state-in-use-layout-effect.ts | 235 +++++ packages/plugins/eslint-plugin/src/index.ts | 1 + website/next.config.mjs | 5 - website/pages/docs/rules/_meta.ts | 1 + ...-direct-set-state-in-use-layout-effect.mdx | 244 ++++++ website/pages/docs/rules/overview.md | 15 +- 10 files changed, 1319 insertions(+), 80 deletions(-) create mode 100644 packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-layout-effect.spec.ts create mode 100644 packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-layout-effect.ts create mode 100644 website/pages/docs/rules/hooks-extra-no-direct-set-state-in-use-layout-effect.mdx 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 b05e538fb..ecd0b4d1a 100644 --- a/packages/plugins/eslint-plugin-react-hooks-extra/src/index.ts +++ b/packages/plugins/eslint-plugin-react-hooks-extra/src/index.ts @@ -1,6 +1,7 @@ /* eslint-disable perfectionist/sort-objects */ import { name, version } from "../package.json"; import noDirectSetStateInUseEffect from "./rules/no-direct-set-state-in-use-effect"; +import noDirectSetStateInUseLayoutEffect from "./rules/no-direct-set-state-in-use-layout-effect"; import noRedundantCustomHook from "./rules/no-redundant-custom-hook"; import ensureUseCallbackHasNonEmptyDeps from "./rules/no-unnecessary-use-callback"; import noUnnecessaryUseMemo from "./rules/no-unnecessary-use-memo"; @@ -13,6 +14,7 @@ export default { }, rules: { "no-direct-set-state-in-use-effect": noDirectSetStateInUseEffect, + "no-direct-set-state-in-use-layout-effect": noDirectSetStateInUseLayoutEffect, "no-redundant-custom-hook": noRedundantCustomHook, "no-unnecessary-use-callback": ensureUseCallbackHasNonEmptyDeps, "no-unnecessary-use-memo": noUnnecessaryUseMemo, @@ -22,7 +24,5 @@ export default { "ensure-custom-hooks-using-other-hooks": noRedundantCustomHook, /** @deprecated Use `no-unnecessary-use-memo` instead */ "ensure-use-memo-has-non-empty-deps": noUnnecessaryUseMemo, - /** @deprecated Use `no-direct-set-state-in-use-effect` instead */ - "no-direct-set-state-in-use-layout-effect": noDirectSetStateInUseEffect, }, } 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 index eabcfbc16..f92eada9d 100644 --- 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 @@ -19,61 +19,6 @@ ruleTester.run(RULE_NAME, rule, { { messageId: "noDirectSetStateInUseEffect" }, ], }, - { - code: /* tsx */ ` - import { useLayoutEffect, useState } from "react"; - - function Component() { - const [data, setData] = useState(0); - useLayoutEffect(() => { - setData(1); - }, []); - return null; - } - `, - errors: [ - { messageId: "noDirectSetStateInUseEffect" }, - ], - }, - { - code: /* tsx */ ` - import { useInsertionEffect, useState } from "react"; - - function Component() { - const [data, setData] = useState(0); - useInsertionEffect(() => { - setData(1); - }, []); - return null; - } - `, - errors: [ - { messageId: "noDirectSetStateInUseEffect" }, - ], - }, - { - code: /* tsx */ ` - import { useState } from "react"; - - function Component() { - const [data, setData] = useState(0); - useIsomorphicLayoutEffect(() => { - setData(1); - }, []); - return null; - } - `, - errors: [ - { messageId: "noDirectSetStateInUseEffect" }, - ], - settings: { - "react-x": { - additionalHooks: { - useLayoutEffect: ["useIsomorphicLayoutEffect"], - }, - }, - }, - }, { code: /* tsx */ ` import { useEffect, useState } from "react"; @@ -825,5 +770,20 @@ ruleTester.run(RULE_NAME, rule, { }, [handlerWatcher]) } `, + /* tsx */ ` + import { useLayoutEffect, useState, useRef } from "react"; + + function Tooltip() { + const ref = useRef(null); + const [tooltipHeight, setTooltipHeight] = useState(0); // You don't know real height yet + + useLayoutEffect(() => { + const { height } = ref.current.getBoundingClientRect(); + setTooltipHeight(height); // Re-render now that you know the real height + }, []); + + // ...use tooltipHeight in the rendering logic below... + } + `, ], }); 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 index 962565322..70aa57b0c 100644 --- 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 @@ -40,17 +40,7 @@ export default createRule<[], MessageID>({ if (!/use\w*Effect/u.test(context.sourceCode.text)) return {}; const settings = decodeSettings(context.settings); const additionalHooks = settings.additionalHooks ?? {}; - const isUseEffectLikeCall = isReactHookCallWithNameAlias( - "useEffect", - context, - [ - "useLayoutEffect", - "useInsertionEffect", - ...additionalHooks.useEffect ?? [], - ...additionalHooks.useLayoutEffect ?? [], - ...additionalHooks.useInsertionEffect ?? [], - ], - ); + const isUseEffectLikeCall = isReactHookCallWithNameAlias("useEffect", context, additionalHooks.useEffect ?? []); const isUseStateCall = isReactHookCallWithNameAlias("useState", context, additionalHooks.useState ?? []); const isUseMemoCall = isReactHookCallWithNameAlias("useMemo", context, additionalHooks.useMemo ?? []); const isUseCallbackCall = isReactHookCallWithNameAlias("useCallback", context, additionalHooks.useCallback ?? []); diff --git a/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-layout-effect.spec.ts b/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-layout-effect.spec.ts new file mode 100644 index 000000000..67a4cc618 --- /dev/null +++ b/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-layout-effect.spec.ts @@ -0,0 +1,812 @@ +import { allValid, ruleTester } from "../../../../../test"; +import rule, { RULE_NAME } from "./no-direct-set-state-in-use-layout-effect"; + +ruleTester.run(RULE_NAME, rule, { + invalid: [ + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function Component() { + const [data, setData] = useState(0); + useLayoutEffect(() => { + setData(1); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useState } from "react"; + + function Component() { + const [data, setData] = useState(0); + useIsomorphicLayoutEffect(() => { + setData(1); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + settings: { + "react-x": { + additionalHooks: { + useLayoutEffect: ["useIsomorphicLayoutEffect"], + }, + }, + }, + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function Component() { + const data = useState(0); + useLayoutEffect(() => { + data[1](); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function Component() { + const data = useState(0); + useLayoutEffect(() => { + data.at(1)(); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const index = 1; + function Component() { + const data = useState(0); + useLayoutEffect(() => { + data.at(index)(); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const index = 1; + function Component() { + const data = useState(0); + useLayoutEffect(() => { + data[index](); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect } from "react"; + + const index = 1; + function Component() { + const data = useCustomState(0); + useLayoutEffect(() => { + data[index](); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + settings: { + "react-x": { + additionalHooks: { + useState: ["useCustomState"], + }, + }, + }, + }, + { + code: /* tsx */ ` + import { useState } from "react"; + + const index = 1; + function Component() { + const data = useState(0); + useCustomEffect(() => { + data[index](); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + settings: { + "react-x": { + additionalHooks: { + useLayoutEffect: ["useCustomEffect"], + }, + }, + }, + }, + { + code: /* tsx */ ` + const index = 1; + function Component() { + const data = useCustomState(0); + useCustomEffect(() => { + data[index](); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + settings: { + "react-x": { + additionalHooks: { + useLayoutEffect: ["useCustomEffect"], + useState: ["useCustomState"], + }, + }, + }, + }, + { + code: /* tsx */ ` + const index = 1; + function Component() { + const data = useCustomState(0); + useCustomEffect(() => { + data.at(index)(); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + settings: { + "react-x": { + additionalHooks: { + useLayoutEffect: ["useCustomEffect"], + useState: ["useCustomState"], + }, + }, + }, + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function Component() { + const [data, setData] = useState(0); + useLayoutEffect(() => { + if (data === 0) { + setData(1); + } + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useLayoutEffect(() => { + const onLoad = () => { + setData(); + }; + onLoad(); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data1, setData1] = useState(); + const [data2, setData2] = useState(); + const setAll = () => { + setData1(); + setData2(); + } + useLayoutEffect(() => { + setAll(); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect", data: { name: "setData1" } }, + { messageId: "noDirectSetStateInUseLayoutEffect", data: { name: "setData2" } }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState, useCallback } from "react"; + + const Component = () => { + const [data1, setData1] = useState(); + const [data2, setData2] = useState(); + const setAll = useCallback(() => { + setData1(); + setData2(); + }) + useLayoutEffect(() => { + setAll(); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect", data: { name: "setData1" } }, + { messageId: "noDirectSetStateInUseLayoutEffect", data: { name: "setData2" } }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useLayoutEffect(() => { + (() => { setData() })(); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useLayoutEffect(() => { + !(function onLoad() { + setData() + })(); + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useLayoutEffect(() => { + const setAll = () => { + setData(); + } + setAll() + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState, useCallback } from "react"; + + const Component = () => { + const [data, setData] = useState(); + const setAll = useCallback(() => setData(), []); + useLayoutEffect(() => { + setAll() + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState, useMemo } from "react"; + + const Component = () => { + const [data, setData] = useState(); + const setAll = useMemo(() => () => setData(), []); + useLayoutEffect(() => { + setAll() + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState, useCallback } from "react"; + + const Component = () => { + const [data, setData] = useState(); + const setAll = useCallback(setData, []); + useLayoutEffect(() => { + setAll() + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState, useMemo } from "react"; + + const Component = () => { + const [data, setData] = useState(); + const setAll = useMemo(() => setData, []); + useLayoutEffect(() => { + setAll() + }, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState, useMemo } from "react"; + + const Component = () => { + const [data, setData] = useState(); + const setAll = useMemo(() => setData, []); + useLayoutEffect(setAll, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + // TODO: Add cleanup function check + // { + // code: /* tsx */ ` + // import { useLayoutEffect, useState } from "react"; + + // const Component = () => { + // const [data, setData] = useState(); + // useLayoutEffect(() => { + // return () => { + // setData(); + // } + // }, []); + // return null; + // } + // `, + // errors: [ + // { messageId: "noDirectSetStateInUseLayoutEffect" }, + // ], + // }, + // TODO: Add cleanup function check + // { + // code: /* tsx */ ` + // import { useLayoutEffect, useState } from "react"; + + // const Component = () => { + // const [data, setData] = useState(); + // useLayoutEffect(() => { + // const cleanup = () => { + // setData(); + // } + // return cleanup; + // }, []); + // return null; + // } + // `, + // errors: [ + // { messageId: "noDirectSetStateInUseLayoutEffect" }, + // ], + // }, + // TODO: Add cleanup function check + // { + // code: /* tsx */ ` + // import { useLayoutEffect, useState } from "react"; + + // const Component = () => { + // const [data, setData] = useState(); + // useLayoutEffect(() => setData, []); + // return null; + // } + // `, + // errors: [ + // { messageId: "noDirectSetStateInUseLayoutEffect" }, + // ], + // }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useLayoutEffect(() => setData(), []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useLayoutEffect(setData, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + const setupFunction = () => { + setData() + } + useLayoutEffect(setupFunction, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + function setupFunction() { + setData() + } + useLayoutEffect(setupFunction, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useLayoutEffect(setupFunction, []); + function setupFunction() { + setData() + } + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component1 = () => { + const [data, setData] = useState(); + const setupFunction = () => { + setData() + } + useLayoutEffect(setupFunction, []); + return null; + } + + const Component2 = () => { + const [data, setData] = useState(); + const setupFunction = () => { + setData() + } + useLayoutEffect(setupFunction, []); + return null; + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + { + code: /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function useCustomHook() { + const [data, setData] = useState(); + const handlerWatcher = () => { + setData() + } + useLayoutEffect(() => { + const abortController = new AbortController() + new MutationObserverWatcher(searchAvatarMetaSelector()) + .addListener('onChange', handlerWatcher) + .startWatch( + { + childList: true, + subtree: true, + attributes: true, + attributeFilter: ['src'], + }, + abortController.signal, + ) + handlerWatcher(); + return () => abortController.abort() + }, [handlerWatcher]) + } + `, + errors: [ + { messageId: "noDirectSetStateInUseLayoutEffect" }, + ], + }, + ], + valid: [ + ...allValid, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function Component() { + const [fn] = useState(() => () => "Function"); + // ... + useLayoutEffect(() => { + fn(); + }, []); + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function Component() { + const [data, setData] = useState(0); + useLayoutEffect(() => { + const handler = () => setData(1); + }, []); + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function Component() { + const [data, setData] = useState(0); + useLayoutEffect(() => { + fetch().then(() => setData()); + }, []); + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data, setData] = useState(); + useLayoutEffect(() => { + const onLoad = () => { + setData(); + }; + }, []); + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const index = 0; + function Component() { + const data = useState(() => 0); + useLayoutEffect(() => { + data.at(index)(); + }, []); + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const index = 0; + function Component() { + const [data, setData] = useState(() => 0); + useLayoutEffect(() => { + void async function () { + const ret = await fetch("https://example.com"); + setData(ret); + }() + }, []); + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data1, setData1] = useState(); + const [data2, setData2] = useState(); + const setAll = () => { + setData1(); + setData2(); + } + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data1, setData1] = useState(); + const [data2, setData2] = useState(); + const setAll = () => { + setData1(); + setData2(); + } + const handler = () => { + setAll(); + } + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component = () => { + const [data1, setData1] = useState(); + const [data2, setData2] = useState(); + function handler() { + setAll(); + } + function setAll() { + setData1(); + setData2(); + } + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component1 = () => { + const [data, setData] = useState(); + const setupFunction = () => { + setData() + } + return null; + } + + const Component2 = () => { + const [data, setData] = useState(); + useLayoutEffect(setupFunction, []); + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + const Component1 = () => { + const [data, setData] = useState(); + const setAll = () => { + setData(); + } + return null; + } + + const Component2 = () => { + const [data, setData] = useState(); + useLayoutEffect(() => { + setAll(); + }, []); + return null; + } + `, + /* tsx */ ` + import { useLayoutEffect, useState } from "react"; + + function useCustomHook() { + const [data, setData] = useState(); + const handlerWatcher = () => { + setData() + } + useLayoutEffect(() => { + const abortController = new AbortController() + new MutationObserverWatcher(searchAvatarMetaSelector()) + .addListener('onChange', handlerWatcher) + .startWatch( + { + childList: true, + subtree: true, + attributes: true, + attributeFilter: ['src'], + }, + abortController.signal, + ) + return () => abortController.abort() + }, [handlerWatcher]) + } + `, + /* tsx */ ` + import { useEffect, useState, useRef } from "react"; + + function Tooltip() { + const ref = useRef(null); + const [tooltipHeight, setTooltipHeight] = useState(0); // You don't know real height yet + + useEffect(() => { + const { height } = ref.current.getBoundingClientRect(); + setTooltipHeight(height); // Re-render now that you know the real height + }, []); + + // ...use tooltipHeight in the rendering logic below... + } + `, + ], +}); diff --git a/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-layout-effect.ts b/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-layout-effect.ts new file mode 100644 index 000000000..dcc6f464f --- /dev/null +++ b/packages/plugins/eslint-plugin-react-hooks-extra/src/rules/no-direct-set-state-in-use-layout-effect.ts @@ -0,0 +1,235 @@ +/* eslint-disable better-mutation/no-mutating-methods */ +import * as AST from "@eslint-react/ast"; +import { isReactHookCallWithNameAlias } from "@eslint-react/core"; +import { decodeSettings } from "@eslint-react/shared"; +import { F, MutRef, O } from "@eslint-react/tools"; +import * as VAR from "@eslint-react/var"; +import { AST_NODE_TYPES } from "@typescript-eslint/types"; +import type { TSESTree } from "@typescript-eslint/utils"; +import type { Scope } from "@typescript-eslint/utils/ts-eslint"; +import type { CamelCase } from "string-ts"; +import { match } from "ts-pattern"; + +import { + createRule, + isFromUseStateCall, + isSetFunctionCall, + isThenCall, + isVariableDeclaratorFromHookCall, +} from "../utils"; + +export const RULE_NAME = "no-direct-set-state-in-use-layout-effect"; + +type MessageID = CamelCase<typeof RULE_NAME>; +type CallKind = "other" | "setState" | "then" | "useLayoutEffect" | "useState"; +type FunctionKind = "cleanup" | "deferred" | "immediate" | "other" | "setup"; + +export default createRule<[], MessageID>({ + meta: { + type: "problem", + docs: { + description: "disallow direct calls to the 'set' function of 'useState' in 'useLayoutEffect'", + }, + messages: { + noDirectSetStateInUseLayoutEffect: + "Do not call the 'set' function '{{name}}' of 'useState' directly in 'useLayoutEffect'.", + }, + schema: [], + }, + name: RULE_NAME, + create(context) { + if (!/use\w*Effect/u.test(context.sourceCode.text)) return {}; + const settings = decodeSettings(context.settings); + const additionalHooks = settings.additionalHooks ?? {}; + const isUseEffectLikeCall = isReactHookCallWithNameAlias( + "useLayoutEffect", + context, + additionalHooks.useLayoutEffect ?? [], + ); + const isUseStateCall = isReactHookCallWithNameAlias("useState", context, additionalHooks.useState ?? []); + const isUseMemoCall = isReactHookCallWithNameAlias("useMemo", context, additionalHooks.useMemo ?? []); + const isUseCallbackCall = isReactHookCallWithNameAlias("useCallback", context, additionalHooks.useCallback ?? []); + const isSetStateCall = isSetFunctionCall(context, settings); + const isIdFromUseStateCall = isFromUseStateCall(context, settings); + const functionStack: [node: AST.TSESTreeFunction, kind: FunctionKind][] = []; + const setupFunctionRef = MutRef.make<AST.TSESTreeFunction | null>(null); + const setupFunctionIdentifiers: TSESTree.Identifier[] = []; + const indirectFunctionCalls: TSESTree.CallExpression[] = []; + const indirectSetStateCalls = new WeakMap<AST.TSESTreeFunction, TSESTree.CallExpression[]>(); + const indirectSetStateCallsAsArgs = new WeakMap<TSESTree.CallExpression, TSESTree.Identifier[]>(); + const indirectSetStateCallsAsSetups = new Map<TSESTree.CallExpression, TSESTree.Identifier[]>(); + const indirectSetStateCallsInHooks = new WeakMap< + TSESTree.VariableDeclarator["init"] & {}, + TSESTree.CallExpression[] + >(); + const onSetupFunctionEnter = (node: AST.TSESTreeFunction) => { + MutRef.set(setupFunctionRef, node); + }; + const onSetupFunctionExit = (node: AST.TSESTreeFunction) => { + MutRef.update(setupFunctionRef, (current) => current === node ? null : current); + }; + function isSetupFunction(node: TSESTree.Node) { + return node.parent?.type === AST_NODE_TYPES.CallExpression + && node.parent.callee !== node + && isUseEffectLikeCall(node.parent); + } + function getCallKind(node: TSESTree.CallExpression) { + return match<TSESTree.CallExpression, CallKind>(node) + .when(isUseStateCall, () => "useState") + .when(isUseEffectLikeCall, () => "useLayoutEffect") + .when(isSetStateCall, () => "setState") + .when(isThenCall, () => "then") + .otherwise(() => "other"); + } + function getFunctionKind(node: AST.TSESTreeFunction) { + return match<AST.TSESTreeFunction, FunctionKind>(node) + .when(isSetupFunction, () => "setup") + .when(AST.isFunctionOfImmediatelyInvoked, () => "immediate") + .otherwise(() => "other"); + } + return { + ":function"(node: AST.TSESTreeFunction) { + const functionKind = getFunctionKind(node); + functionStack.push([node, functionKind]); + if (functionKind === "setup") onSetupFunctionEnter(node); + }, + ":function:exit"(node: AST.TSESTreeFunction) { + const [_, functionKind] = functionStack.at(-1) ?? []; + functionStack.pop(); + if (functionKind === "setup") onSetupFunctionExit(node); + }, + CallExpression(node) { + const effectFn = MutRef.get(setupFunctionRef); + const [parentFn, parentFnKind] = functionStack.at(-1) ?? []; + if (parentFn?.async) return; + match(getCallKind(node)) + .with("setState", () => { + if (!parentFn) return; + if (parentFn !== effectFn && parentFnKind !== "immediate") { + const maybeVd = AST.traverseUpGuard(node, isVariableDeclaratorFromHookCall); + if (O.isSome(maybeVd)) { + const vd = maybeVd.value; + const calls = indirectSetStateCallsInHooks.get(vd.init) ?? []; + indirectSetStateCallsInHooks.set(vd.init, [...calls, node]); + return; + } + const calls = indirectSetStateCalls.get(parentFn) ?? []; + indirectSetStateCalls.set(parentFn, [...calls, node]); + return; + } + context.report({ + messageId: "noDirectSetStateInUseLayoutEffect", + node, + data: { name: context.sourceCode.getText(node.callee) }, + }); + }) + // .with(P.union("useMemo", "useCallback"), () => {}) + .with("useLayoutEffect", () => { + const [firstArg] = node.arguments; + if (AST.isFunction(firstArg)) return; + const identifiers = AST.getNestedIdentifiers(node); + setupFunctionIdentifiers.push(...identifiers); + }) + .with("other", () => { + const isInSetupFunction = effectFn === parentFn; + if (!isInSetupFunction) return; + indirectFunctionCalls.push(node); + }) + .otherwise(F.constVoid); + }, + Identifier(node) { + if (node.parent.type === AST_NODE_TYPES.CallExpression && node.parent.callee === node) return; + if (!isIdFromUseStateCall(node)) return; + switch (node.parent.type) { + case AST_NODE_TYPES.CallExpression: { + const [firstArg] = node.parent.arguments; + if (node !== firstArg) break; + // const [state, setState] = useState(); + // const set = useCallback(setState, []); + // useLayoutEffect(set, []); + if (isUseCallbackCall(node.parent)) { + const maybeVd = AST.traverseUpGuard(node.parent, isVariableDeclaratorFromHookCall); + if (O.isNone(maybeVd)) break; + const vd = maybeVd.value; + const calls = indirectSetStateCallsAsArgs.get(vd.init) ?? []; + indirectSetStateCallsAsArgs.set(vd.init, [...calls, node]); + } + // const [state, setState] = useState(); + // useLayoutEffect(setState); + if (isUseEffectLikeCall(node.parent)) { + const calls = indirectSetStateCallsAsArgs.get(node.parent) ?? []; + indirectSetStateCallsAsSetups.set(node.parent, [...calls, node]); + } + break; + } + case AST_NODE_TYPES.ArrowFunctionExpression: { + const parent = node.parent.parent; + if (parent.type !== AST_NODE_TYPES.CallExpression) break; + // const [state, setState] = useState(); + // const set = useMemo(() => setState, []); + // useLayoutEffect(set, []); + if (!isUseMemoCall(parent)) break; + const maybeVd = AST.traverseUpGuard(parent, isVariableDeclaratorFromHookCall); + if (O.isNone(maybeVd)) break; + const vd = maybeVd.value; + const calls = indirectSetStateCallsAsArgs.get(vd.init) ?? []; + indirectSetStateCallsAsArgs.set(vd.init, [...calls, node]); + } + } + }, + "Program:exit"() { + const getSetStateCalls = ( + id: TSESTree.Identifier | string, + initialScope: Scope.Scope, + ): TSESTree.CallExpression[] | TSESTree.Identifier[] => { + const node = O.flatMap(VAR.findVariable(id, initialScope), VAR.getVariableNode(0)).pipe(O.getOrNull); + switch (node?.type) { + case AST_NODE_TYPES.FunctionDeclaration: + case AST_NODE_TYPES.FunctionExpression: + case AST_NODE_TYPES.ArrowFunctionExpression: + return indirectSetStateCalls.get(node) ?? []; + case AST_NODE_TYPES.CallExpression: + return indirectSetStateCallsInHooks.get(node) ?? indirectSetStateCallsAsArgs.get(node) ?? []; + } + return []; + }; + for (const [_, calls] of indirectSetStateCallsAsSetups) { + for (const call of calls) { + context.report({ + messageId: "noDirectSetStateInUseLayoutEffect", + node: call, + data: { name: call.name }, + }); + } + } + for (const { callee } of indirectFunctionCalls) { + if (!("name" in callee)) continue; + const { name } = callee; + const setStateCalls = getSetStateCalls(name, context.sourceCode.getScope(callee)); + for (const setStateCall of setStateCalls) { + context.report({ + messageId: "noDirectSetStateInUseLayoutEffect", + node: setStateCall, + data: { + name: AST.toReadableNodeName(setStateCall, (n) => context.sourceCode.getText(n)), + }, + }); + } + } + for (const id of setupFunctionIdentifiers) { + const setStateCalls = getSetStateCalls(id.name, context.sourceCode.getScope(id)); + for (const setStateCall of setStateCalls) { + context.report({ + messageId: "noDirectSetStateInUseLayoutEffect", + node: setStateCall, + data: { + name: AST.toReadableNodeName(setStateCall, (n) => context.sourceCode.getText(n)), + }, + }); + } + } + }, + }; + }, + defaultOptions: [], +}); diff --git a/packages/plugins/eslint-plugin/src/index.ts b/packages/plugins/eslint-plugin/src/index.ts index 01667199c..830845318 100644 --- a/packages/plugins/eslint-plugin/src/index.ts +++ b/packages/plugins/eslint-plugin/src/index.ts @@ -83,6 +83,7 @@ const allPreset = { // Part: Hooks Extra "hooks-extra/no-direct-set-state-in-use-effect": "warn", + "hooks-extra/no-direct-set-state-in-use-layout-effect": "warn", "hooks-extra/no-redundant-custom-hook": "warn", "hooks-extra/no-unnecessary-use-callback": "warn", "hooks-extra/no-unnecessary-use-memo": "warn", diff --git a/website/next.config.mjs b/website/next.config.mjs index c76ec0721..6d27eaff0 100644 --- a/website/next.config.mjs +++ b/website/next.config.mjs @@ -63,11 +63,6 @@ const nextConfig = { destination: "/docs/rules/hooks-extra-no-unnecessary-use-callback", permanent: true, }, - { - source: "/docs/rules/hooks-extra-no-direct-set-state-in-use-layout-effect", - destination: "/docs/rules/hooks-extra-no-direct-set-state-in-use-effect", - permanent: true, - }, { source: "/docs/rules/debug-react-hooks", destination: "/docs/rules/debug-hook", diff --git a/website/pages/docs/rules/_meta.ts b/website/pages/docs/rules/_meta.ts index 313385e5d..a49aaf184 100644 --- a/website/pages/docs/rules/_meta.ts +++ b/website/pages/docs/rules/_meta.ts @@ -72,6 +72,7 @@ export default { "hooks-extra-no-unnecessary-use-callback": "hooks-extra/no-unnecessary-use-callback", "hooks-extra-no-unnecessary-use-memo": "hooks-extra/no-unnecessary-use-memo", "hooks-extra-no-direct-set-state-in-use-effect": "hooks-extra/no-direct-set-state-in-use-effect", + "hooks-extra-no-direct-set-state-in-use-layout-effect": "hooks-extra/no-direct-set-state-in-use-layout-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-direct-set-state-in-use-layout-effect.mdx b/website/pages/docs/rules/hooks-extra-no-direct-set-state-in-use-layout-effect.mdx new file mode 100644 index 000000000..08be90f3c --- /dev/null +++ b/website/pages/docs/rules/hooks-extra-no-direct-set-state-in-use-layout-effect.mdx @@ -0,0 +1,244 @@ +import { Info } from "#/components/callout" + +# no-direct-set-state-in-use-layout-effect + +## Rule category + +Correctness. + +## What it does + +<Info> +This rule only checks for **direct** calls to the `set` function of `useState` in `useLayoutEffect`. It does not check for calls to `set` function in callbacks, event handlers, or `Promise.then` functions. +</Info> + +Disallow **direct** calls to the [`set` function](https://react.dev/reference/react/useState#setstate) of `useState` in `useLayoutEffect`. + +## Examples + +<Info> +The first three cases are common valid use cases because they are not called the `set` function directly in `useLayoutEffect`. +</Info> + +### Passing + +```tsx +import { useState, useLayoutEffect } from "react"; + +export default function Counter() { + const [count, setCount] = useState(0); + + useLayoutEffect(() => { + const handler = () => setCount(c => c + 1); + window.addEventListener("click", handler); + return () => window.removeEventListener("click", handler); + }, []); + + return <h1>{count}</h1>; +} +``` + +### Passing + +```tsx +import { useState, useLayoutEffect } from "react"; + +export default function Counter() { + const [count, setCount] = useState(0); + + useLayoutEffect(() => { + const intervalId = setInterval(() => { + setCount(c => c + 1); + }, 1000); + return () => clearInterval(intervalId); + }, []); + + return <h1>{count}</h1>; +} +``` + +### Passing + +```tsx +import { useState, useLayoutEffect } from "react"; + +export default function RemoteContent() { + const [content, setContent] = useState(""); + + useLayoutEffect(() => { + let discarded = false; + fetch("https://example.com/content") + .then(resp => resp.text()) + .then(text => { + if (discarded) return; + setContent(text); + }); + return () => { + discarded = true; + }; + }, []); + + return <h1>{count}</h1>; +} +``` + +### Failing + +```tsx +import { useLayoutEffect, useState } from 'react'; + +function Form() { + const [firstName, setFirstName] = useState('Taylor'); + const [lastName, setLastName] = useState('Swift'); + + // 🔴 Avoid: redundant state and unnecessary Effect + const [fullName, setFullName] = useState(''); + useLayoutEffect(() => { + setFullName(firstName + ' ' + lastName); + }, [firstName, lastName]); + // ... +} +``` + +### Passing + +```tsx +import { useState } from 'react'; + +function Form() { + const [firstName, setFirstName] = useState('Taylor'); + const [lastName, setLastName] = useState('Swift'); + // ✅ Good: calculated during rendering + const fullName = firstName + ' ' + lastName; + // ... +} +``` + +### Failing + +```tsx +import { useLayoutEffect, useState } from 'react'; + +function TodoList({ todos, filter }) { + const [newTodo, setNewTodo] = useState(''); + + // 🔴 Avoid: redundant state and unnecessary Effect + const [visibleTodos, setVisibleTodos] = useState([]); + useLayoutEffect(() => { + setVisibleTodos(getFilteredTodos(todos, filter)); + }, [todos, filter]); + + // ... +} +``` + +### Passing + +```tsx +import { useMemo, useState } from 'react'; + +function TodoList({ todos, filter }) { + const [newTodo, setNewTodo] = useState(''); + // ✅ Does not re-run getFilteredTodos() unless todos or filter change + const visibleTodos = useMemo(() => getFilteredTodos(todos, filter), [todos, filter]); + // ... +} +``` + +### Failing + +```tsx +import { useLayoutEffect, useState } from 'react'; + +export default function ProfilePage({ userId }) { + const [comment, setComment] = useState(''); + + // 🔴 Avoid: Resetting state on prop change in an Effect + useLayoutEffect(() => { + setComment(''); + }, [userId]); + // ... +} +``` + +### Passing + +```tsx +import { useState } from 'react'; + +export default function ProfilePage({ userId }) { + return ( + <Profile + userId={userId} + key={userId} + /> + ); +} + +function Profile({ userId }) { + // ✅ This and any other state below will reset on key change automatically + const [comment, setComment] = useState(''); + // ... +} +``` + +### Failing + +```tsx +import { useLayoutEffect, useState } from 'react'; + +function List({ items }) { + const [isReverse, setIsReverse] = useState(false); + const [selection, setSelection] = useState(null); + + // 🔴 Avoid: Adjusting state on prop change in an Effect + useLayoutEffect(() => { + setSelection(null); + }, [items]); + // ... +} +``` + +### Passing + +```tsx +import { useState } from 'react'; + +function List({ items }) { + const [isReverse, setIsReverse] = useState(false); + const [selection, setSelection] = useState(null); + + // Better: Adjust the state while rendering + const [prevItems, setPrevItems] = useState(items); + if (items !== prevItems) { + setPrevItems(items); + setSelection(null); + } + // ... +} +``` + +```tsx +import { useState } from 'react'; + +function List({ items }) { + const [isReverse, setIsReverse] = useState(false); + const [selectedId, setSelectedId] = useState(null); + // ✅ Best: Calculate everything during rendering + const selection = items.find(item => item.id === selectedId) ?? null; + // ... +} +``` + +## Known limitations + +- The `set` call to `useState` in the `cleanup` function of `useLayoutEffect` will not be checked. +- The current implementation does not support determining whether a `set` function called in an `async` function is actually at least one `await` after. + +The limitation may be lifted in the future. + +### Further Reading + +- [React: useState](https://react.dev/reference/react/useState#setstate) +- [React: useLayoutEffect](https://react.dev/reference/react/useLayoutEffect) +- [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 a805af639..e73db1efe 100644 --- a/website/pages/docs/rules/overview.md +++ b/website/pages/docs/rules/overview.md @@ -97,13 +97,14 @@ ## Hooks Extra Rules -| Rule | Description | 💼 | 💭 | | -| :----------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------ | :-: | :-: | :-: | -| [`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/no-redundant-custom-hook`](hooks-extra-no-redundant-custom-hook) | Warns when custom Hooks that don't use other Hooks. | ✔️ | | | -| [`hooks-extra/no-unnecessary-use-callback`](hooks-extra-no-unnecessary-use-callback) | Disallow unnecessary usage of `useCallback`. | ✔️ | | 📐 | -| [`hooks-extra/no-unnecessary-use-memo`](hooks-extra-no-unnecessary-use-memo) | Disallow unnecessary usage of `useMemo`. | ✔️ | | 📐 | -| [`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/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/no-direct-set-state-in-use-layout-effect`](hooks-extra-no-direct-set-state-in-use-layout-effect) | Disallow direct calls to the `set` function of `useState` in `useLayoutEffect`. | ✔️ | | 📐 | +| [`hooks-extra/no-redundant-custom-hook`](hooks-extra-no-redundant-custom-hook) | Warns when custom Hooks that don't use other Hooks. | ✔️ | | | +| [`hooks-extra/no-unnecessary-use-callback`](hooks-extra-no-unnecessary-use-callback) | Disallow unnecessary usage of `useCallback`. | ✔️ | | 📐 | +| [`hooks-extra/no-unnecessary-use-memo`](hooks-extra-no-unnecessary-use-memo) | Disallow unnecessary usage of `useMemo`. | ✔️ | | 📐 | +| [`hooks-extra/prefer-use-state-lazy-initialization`](hooks-extra-prefer-use-state-lazy-initialization) | Warns function calls made inside `useState` calls. | 🚀 | | | ## Naming Convention Rules