diff --git a/packages/@romejs/diagnostics/categories.ts b/packages/@romejs/diagnostics/categories.ts index 5f3f744f956..a18cac5e559 100644 --- a/packages/@romejs/diagnostics/categories.ts +++ b/packages/@romejs/diagnostics/categories.ts @@ -124,6 +124,7 @@ type LintDiagnosticCategory = | "lint/noDebugger" | "lint/noDelete" | "lint/noDeleteVars" + | "lint/noDidUpdateSetState" | "lint/noDupeArgs" | "lint/noDuplicateCase" | "lint/noDuplicateGroupNamesInRegularExpressions" diff --git a/packages/@romejs/diagnostics/descriptions.ts b/packages/@romejs/diagnostics/descriptions.ts index 632dfd868d8..13d5c5f7beb 100644 --- a/packages/@romejs/diagnostics/descriptions.ts +++ b/packages/@romejs/diagnostics/descriptions.ts @@ -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.", @@ -1687,4 +1690,4 @@ export const descriptions = createMessages({ }), RECURSIVE_CONFIG: "Recursive config", }, -}); +}); \ No newline at end of file diff --git a/packages/@romejs/js-compiler/lint/rules/index.ts b/packages/@romejs/js-compiler/lint/rules/index.ts index fcbde619e74..9bd04c5494e 100644 --- a/packages/@romejs/js-compiler/lint/rules/index.ts +++ b/packages/@romejs/js-compiler/lint/rules/index.ts @@ -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"; @@ -106,6 +107,7 @@ export const lintTransforms = [ noDebugger, noDelete, noDeleteVars, + noDidUpdateSetState, noDupeArgs, noDuplicateCase, noDuplicateGroupNamesInRegularExpressions, diff --git a/packages/@romejs/js-compiler/lint/rules/react/.jsxA11yAltText.ts.swp b/packages/@romejs/js-compiler/lint/rules/react/.jsxA11yAltText.ts.swp new file mode 100644 index 00000000000..21c1b2f64c2 Binary files /dev/null and b/packages/@romejs/js-compiler/lint/rules/react/.jsxA11yAltText.ts.swp differ diff --git a/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.test.md b/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.test.md new file mode 100644 index 00000000000..f10f4383ec5 --- /dev/null +++ b/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.test.md @@ -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", + }); + } + } +} + +``` diff --git a/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.test.ts b/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.test.ts new file mode 100644 index 00000000000..65bec649b58 --- /dev/null +++ b/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.test.ts @@ -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"}, + ); + }, +); diff --git a/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.ts b/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.ts new file mode 100644 index 00000000000..8684b58fa05 --- /dev/null +++ b/packages/@romejs/js-compiler/lint/rules/react/noDidUpdateSetState.ts @@ -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; + }, +};