Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
Merge pull request #433 from VictorHom/noDidUpdateSetState
Browse files Browse the repository at this point in the history
add rule for no setState in componentDidUpdate and tests
  • Loading branch information
sebmck authored May 17, 2020
2 parents 93e2f3c + 3167738 commit b37cea8
Show file tree
Hide file tree
Showing 7 changed files with 275 additions and 1 deletion.
1 change: 1 addition & 0 deletions packages/@romejs/diagnostics/categories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ type LintDiagnosticCategory =
| "lint/noDebugger"
| "lint/noDelete"
| "lint/noDeleteVars"
| "lint/noDidUpdateSetState"
| "lint/noDupeArgs"
| "lint/noDuplicateCase"
| "lint/noDuplicateGroupNamesInRegularExpressions"
Expand Down
5 changes: 4 additions & 1 deletion packages/@romejs/diagnostics/descriptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,9 @@ export const descriptions = createMessages({
},
// @romejs/js-compiler
LINT: {
NO_DID_UPDATE_SET_STATE: {
category: "lint/noDidUpdateSetState",
message: "Avoid this.setState in componentDidUpdate",
JSX_A11Y_HEADING_HAS_CONTENT: {
category: "lint/jsxA11yHeadingHasContent",
message: "Headings must have content and the content must be accessible by a screen reader.",
Expand Down Expand Up @@ -1687,4 +1690,4 @@ export const descriptions = createMessages({
}),
RECURSIVE_CONFIG: "Recursive config",
},
});
});
2 changes: 2 additions & 0 deletions packages/@romejs/js-compiler/lint/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import noDangerWithChildren from "./react/noDangerWithChildren";
import noDebugger from "./regular/noDebugger";
import noDelete from "./regular/noDelete";
import noDeleteVars from "./regular/noDeleteVars";
import noDidUpdateSetState from "./react/noDidUpdateSetState";
import noDupeArgs from "./regular/noDupeArgs";
import noDuplicateCase from "./regular/noDuplicateCase";
import noDuplicateGroupNamesInRegularExpressions from "./regular/noDuplicateGroupNamesInRegularExpressions";
Expand Down Expand Up @@ -106,6 +107,7 @@ export const lintTransforms = [
noDebugger,
noDelete,
noDeleteVars,
noDidUpdateSetState,
noDupeArgs,
noDuplicateCase,
noDuplicateGroupNamesInRegularExpressions,
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
# `noDidUpdateSetState.test.ts`

**DO NOT MODIFY**. This file has been autogenerated. Run `rome test packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.test.ts --update-snapshots` to update.

## `no this.setState in componentDidUpdate`

### `0`

```
unknown:4:12 lint/noDidUpdateSetState ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
✖ Avoid this.setState in componentDidUpdate
2 │ class Hello extends React.Component {
3 │ componentDidUpdate() {
> 4 │ this.setState({
│ ^^^^^^^^^^^^^
5 │ name: 'John'
6 │ });
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
✖ Found 1 problem
```

### `0: formatted`

```
class Hello extends React.Component {
componentDidUpdate() {
this.setState({
name: "John",
});
}
}
```

### `1`

```
unknown:5:12 lint/noDidUpdateSetState ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
✖ Avoid this.setState in componentDidUpdate
3 │ componentDidUpdate() {
4 │ foo();
> 5 │ this.setState({
│ ^^^^^^^^^^^^^
6 │ name: 'John'
7 │ });
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
✖ Found 1 problem
```

### `1: formatted`

```
class Hello extends React.Component {
componentDidUpdate() {
foo();
this.setState({
name: "John",
});
}
}
```

### `2`

```
unknown:4:12 lint/noDidUpdateSetState ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
✖ Avoid this.setState in componentDidUpdate
2 │ class Hello extends Component {
3 │ componentDidUpdate() {
> 4 │ this.setState({
│ ^^^^^^^^^^^^^
5 │ name: 'John'
6 │ });
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
✖ Found 1 problem
```

### `2: formatted`

```
class Hello extends Component {
componentDidUpdate() {
this.setState({
name: "John",
});
}
}
```

### `3`

```
unknown:5:12 lint/noDidUpdateSetState ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
✖ Avoid this.setState in componentDidUpdate
3 │ componentDidUpdate() {
4 │ foo();
> 5 │ this.setState({
│ ^^^^^^^^^^^^^
6 │ name: 'John'
7 │ });
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
✖ Found 1 problem
```

### `3: formatted`

```
class Hello extends Component {
componentDidUpdate() {
foo();
this.setState({
name: "John",
});
}
}
```

### `4`

```
✔ No known problems!
```

### `4: formatted`

```
class Hello extends React.Component {
componentDidUpdate() {
if (condition) {
this.setState({
name: "John",
});
}
}
}
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import {test} from "rome";
import {testLintMultiple} from "../testHelpers";

test(
"no this.setState in componentDidUpdate",
async (t) => {
await testLintMultiple(
t,
[
// INVALID
`
class Hello extends React.Component {
componentDidUpdate() {
this.setState({
name: 'John'
});
}
}
`,
`
class Hello extends React.Component {
componentDidUpdate() {
foo();
this.setState({
name: 'John'
});
}
}
`,
`
class Hello extends Component {
componentDidUpdate() {
this.setState({
name: 'John'
});
}
}
`,
`
class Hello extends Component {
componentDidUpdate() {
foo();
this.setState({
name: 'John'
});
}
}
`,
// VALID
`
class Hello extends React.Component {
componentDidUpdate() {
if (condition) {
this.setState({
name: 'John'
});
}
}
}
`,
],
{category: "lint/noDidUpdateSetState"},
);
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import {Path, TransformExitResult} from "@romejs/js-compiler";
import {descriptions} from "@romejs/diagnostics";
import {doesNodeMatchPattern, isConditional} from "@romejs/js-ast-utils";

function inComponentDidUpdate(path: Path): boolean {
const func = path.findAncestry(({node}) => isConditional(node)) !== undefined
return !func && path.findAncestry(({node}) => node.type === "ClassMethod" && node.key.type === "StaticPropertyKey" && node.key.value.type === "Identifier" && node.key.value.name === "componentDidUpdate") !== undefined;
}

export default {
name: "noDidUpdateSetState",
enter(path: Path): TransformExitResult {
const {node} = path;

if (doesNodeMatchPattern(node, "this.setState") && inComponentDidUpdate(path)) {
path.context.addNodeDiagnostic(
node,
descriptions.LINT.NO_DID_UPDATE_SET_STATE
);
}

return node;
},
};

0 comments on commit b37cea8

Please sign in to comment.