Skip to content

feat: add rule 'no-direct-set-state-in-use-effect', closes #628 #629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions eslint.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,14 @@ const config: FlatConfig[] = [
"dedent",
"html",
"tsx",
"ts",
],
tags: [
"outdent",
"dedent",
"html",
"tsx",
"ts",
],
},
],
Expand Down
13 changes: 7 additions & 6 deletions packages/plugins/eslint-plugin-react-hooks-extra/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. | 🚀 | | |
2 changes: 2 additions & 0 deletions packages/plugins/eslint-plugin-react-hooks-extra/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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;
Original file line number Diff line number Diff line change
@@ -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();
};
}, []);
}
`,
],
});
Original file line number Diff line number Diff line change
@@ -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<typeof RULE_NAME>;

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<MessageID>;
4 changes: 4 additions & 0 deletions website/pages/docs/glossary.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
1 change: 1 addition & 0 deletions website/pages/docs/rules/_meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading