Skip to content

Commit d6c3f5c

Browse files
authored
feat: add rule react/no-direct-mutation-state, closes #202 (#203)
1 parent debe595 commit d6c3f5c

File tree

9 files changed

+259
-1
lines changed

9 files changed

+259
-1
lines changed

CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
1+
## v0.9.3 (Draft)
2+
3+
### Release Notes
4+
5+
#### Add rule `react/no-direct-mutation-state`
6+
7+
---
8+
9+
#### 🏠 Internal
10+
11+
- `@eslint-react/eslint-plugin-react`
12+
- Add rule `react/no-direct-mutation-state`.
13+
14+
#### Authors: 1
15+
16+
- Eva1ent ([@Rel1cx](https://github.com/Rel1cx))
17+
18+
---
19+
120
## v0.9.2 (Wed Dec 6 2023)
221

322
### Release Notes

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ The following presets are available in this plugin:
147147
- [x] `react/no-missing-iframe-sandbox`
148148
- [x] `react/no-missing-component-display-name`
149149
- [x] `react/no-script-url`
150-
- [ ] `react/no-direct-mutation-state`
150+
- [x] `react/no-direct-mutation-state`
151151
- [x] `react/no-redundant-should-component-update`
152152
- [x] `react/no-set-state-in-component-did-mount`
153153
- [x] `react/no-set-state-in-component-did-update`

packages/eslint-plugin-react/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import noConstructedContextValue from "./rules/no-constructed-context-value";
1717
import noCreateRef from "./rules/no-create-ref";
1818
import noDangerouslySetInnerHTML from "./rules/no-dangerously-set-innerhtml";
1919
import noDangerouslySetInnerHTMLWithChildren from "./rules/no-dangerously-set-innerhtml-with-children";
20+
import noDirectMutationState from "./rules/no-direct-mutation-state";
2021
import noFindDomNode from "./rules/no-find-dom-node";
2122
import noMissingButtonType from "./rules/no-missing-button-type";
2223
import noMissingComponentDisplayName from "./rules/no-missing-component-display-name";
@@ -60,6 +61,7 @@ export const rules = {
6061
"no-create-ref": noCreateRef,
6162
"no-dangerously-set-innerhtml": noDangerouslySetInnerHTML,
6263
"no-dangerously-set-innerhtml-with-children": noDangerouslySetInnerHTMLWithChildren,
64+
"no-direct-mutation-state": noDirectMutationState,
6365
"no-find-dom-node": noFindDomNode,
6466
"no-missing-button-type": noMissingButtonType,
6567
"no-missing-component-display-name": noMissingComponentDisplayName,
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# react/no-direct-mutation-state
2+
3+
## Rule category
4+
5+
Correctness.
6+
7+
## What it does
8+
9+
Disallows direct mutation of `this.state`.
10+
11+
## Why is this bad?
12+
13+
NEVER mutate `this.state` directly, as calling `setState()` afterwards may replace the mutation you made. Treat `this.state` as if it were immutable.
14+
15+
The only place that's acceptable to assign `this.state` is in a class component's `constructor`.
16+
17+
## Examples
18+
19+
### ❌ Incorrect
20+
21+
```tsx
22+
import React from "react";
23+
24+
class MyComponent extends React.Component {
25+
state = {
26+
foo: "bar",
27+
};
28+
29+
componentDidMount() {
30+
this.state.foo = "baz";
31+
}
32+
33+
render() {
34+
return <div>{this.state.foo}</div>;
35+
}
36+
}
37+
```
38+
39+
### ✅ Correct
40+
41+
```tsx
42+
import React from "react";
43+
44+
class MyComponent extends React.Component {
45+
constructor(props) {
46+
super(props);
47+
this.state = {
48+
foo: "bar",
49+
};
50+
}
51+
52+
componentDidMount() {
53+
this.setState({ foo: "baz" });
54+
}
55+
56+
render() {
57+
return <div>{this.state.foo}</div>;
58+
}
59+
}
60+
```
61+
62+
```tsx
63+
import React from "react";
64+
65+
class MyComponent extends React.Component {
66+
state = {
67+
foo: "bar",
68+
};
69+
70+
componentDidMount() {
71+
this.setState({ foo: "baz" });
72+
}
73+
74+
render() {
75+
return <div>{this.state.foo}</div>;
76+
}
77+
}
78+
```
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import dedent from "dedent";
2+
3+
import { allValid, defaultParserOptions, RuleTester } from "../../../../test";
4+
import rule, { RULE_NAME } from "./no-direct-mutation-state";
5+
6+
const ruleTester = new RuleTester({
7+
parser: "@typescript-eslint/parser",
8+
parserOptions: defaultParserOptions,
9+
});
10+
11+
ruleTester.run(RULE_NAME, rule, {
12+
valid: [
13+
...allValid,
14+
dedent`
15+
class Foo extends React.Component {
16+
componentDidMount() {
17+
class Bar extends Baz {
18+
componentDidMount() {
19+
this.state = { foo: "bar" };
20+
}
21+
}
22+
}
23+
}
24+
`,
25+
dedent`
26+
class Hello extends React.Component {
27+
constructor(props) {
28+
super(props)
29+
30+
this.state = {
31+
foo: 'bar',
32+
}
33+
}
34+
}
35+
`,
36+
dedent`
37+
import React from "react";
38+
39+
class MyComponent extends React.Component {
40+
state = {
41+
foo: "bar",
42+
};
43+
44+
componentDidMount() {
45+
this.setState({ foo: "baz" });
46+
}
47+
48+
render() {
49+
return <div>{this.state.foo}</div>;
50+
}
51+
}
52+
`,
53+
],
54+
invalid: [
55+
{
56+
code: dedent`
57+
class Hello extends React.Component {
58+
constructor(props) {
59+
super(props)
60+
61+
// Assign at instance creation time, not on a callback
62+
doSomethingAsync(() => {
63+
this.state = 'bad';
64+
});
65+
}
66+
}
67+
`,
68+
errors: [{
69+
messageId: "NO_DIRECT_MUTATION_STATE",
70+
}],
71+
},
72+
],
73+
});
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { isOneOf, NodeType, traverseUpGuard } from "@eslint-react/ast";
2+
import { isClassComponent } from "@eslint-react/core";
3+
import { O } from "@eslint-react/tools";
4+
import type { TSESTree } from "@typescript-eslint/utils";
5+
import { ESLintUtils } from "@typescript-eslint/utils";
6+
import type { ConstantCase } from "string-ts";
7+
8+
import { createRule } from "../utils";
9+
10+
export const RULE_NAME = "no-direct-mutation-state";
11+
12+
export type MessageID = ConstantCase<typeof RULE_NAME>;
13+
14+
function isAssignmentToThisState(node: TSESTree.AssignmentExpression) {
15+
const { left } = node;
16+
17+
return (
18+
left.type === NodeType.MemberExpression
19+
&& left.object.type === NodeType.ThisExpression
20+
&& left.property.type === NodeType.Identifier
21+
&& left.property.name === "state"
22+
);
23+
}
24+
25+
function isConstructorFunction(
26+
node: TSESTree.Node,
27+
): node is TSESTree.FunctionDeclaration | TSESTree.FunctionExpression {
28+
return isOneOf([NodeType.FunctionDeclaration, NodeType.FunctionExpression])(node)
29+
&& isOneOf([NodeType.MethodDefinition, NodeType.PropertyDefinition])(node.parent)
30+
&& node.parent.key.type === NodeType.Identifier
31+
&& node.parent.key.name === "constructor"
32+
&& node.parent.parent.type === NodeType.ClassBody;
33+
}
34+
35+
export default createRule<[], MessageID>({
36+
name: RULE_NAME,
37+
meta: {
38+
type: "problem",
39+
docs: {
40+
description: "disallow direct mutation of `state`",
41+
recommended: "recommended",
42+
requiresTypeChecking: false,
43+
},
44+
schema: [],
45+
messages: {
46+
NO_DIRECT_MUTATION_STATE: "Do not mutate `state` directly. Use `setState` instead.",
47+
},
48+
},
49+
defaultOptions: [],
50+
create(context) {
51+
return {
52+
AssignmentExpression(node) {
53+
if (!isAssignmentToThisState(node)) {
54+
return;
55+
}
56+
57+
const maybeParentClass = traverseUpGuard(node, isOneOf([NodeType.ClassDeclaration, NodeType.ClassExpression]));
58+
59+
if (O.isNone(maybeParentClass)) {
60+
return;
61+
}
62+
63+
const parentClass = maybeParentClass.value;
64+
65+
if (!isClassComponent(parentClass, context)) {
66+
return;
67+
}
68+
69+
const maybeParentConstructor = traverseUpGuard(node, isConstructorFunction);
70+
71+
if (O.exists(maybeParentConstructor, n => context.sourceCode.getScope?.(node).block === n)) {
72+
return;
73+
}
74+
75+
context.report({
76+
node,
77+
messageId: "NO_DIRECT_MUTATION_STATE",
78+
});
79+
},
80+
};
81+
},
82+
});

packages/eslint-plugin/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const rulePreset = {
4848
"react/no-create-ref": "error",
4949
"react/no-dangerously-set-innerhtml": "warn",
5050
"react/no-dangerously-set-innerhtml-with-children": "error",
51+
"react/no-direct-mutation-state": "error",
5152
"react/no-find-dom-node": "error",
5253
"react/no-missing-button-type": "warn",
5354
"react/no-missing-component-display-name": "warn",
@@ -98,6 +99,7 @@ const recommendedPreset = {
9899
"react/no-create-ref": "error",
99100
"react/no-dangerously-set-innerhtml": "warn",
100101
"react/no-dangerously-set-innerhtml-with-children": "error",
102+
"react/no-direct-mutation-state": "error",
101103
"react/no-find-dom-node": "error",
102104
"react/no-missing-button-type": "warn",
103105
"react/no-missing-iframe-sandbox": "warn",

website/pages/rules/_meta.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
"react-no-create-ref": "react/no-create-ref",
3333
"react-no-dangerously-set-innerhtml": "react/no-dangerously-set-innerhtml",
3434
"react-no-dangerously-set-innerhtml-with-children": "react/no-dangerously-set-innerhtml-with-children",
35+
"react-no-direct-mutation-state": "react/no-direct-mutation-state",
3536
"react-no-find-dom-node": "react/no-find-dom-node",
3637
"react-no-missing-button-type": "react/no-missing-button-type",
3738
"react-no-missing-component-display-name": "react/no-missing-component-display-name",

website/pages/rules/overview.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
| [react/no-create-ref](react-no-create-ref) | disallow `createRef` in function components |
4444
| [react/no-dangerously-set-innerhtml](react-no-dangerously-set-innerhtml) | disallow when a DOM element is using `dangerouslySetInnerHTML` |
4545
| [react/no-dangerously-set-innerhtml-with-children](react-no-dangerously-set-innerhtml-with-children) | disallow when a DOM element is using both `children` and `dangerouslySetInnerHTML` |
46+
| [react/no-direct-mutation-state](react-no-direct-mutation-state) | disallow direct mutation of `this.state` |
4647
| [react/no-find-dom-node](react-no-find-dom-node) | disallow `findDOMNode` |
4748
| [react/no-missing-button-type](react-no-missing-button-type) | enforce that `button` elements have an explicit `type` attribute |
4849
| [react/no-missing-iframe-sandbox](react-no-missing-iframe-sandbox) | enforce that `iframe` elements explicitly specify a `sandbox` attribute |

0 commit comments

Comments
 (0)