From 691dc3b1e60203bc2aa58933f05cf0d06357f3a6 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Mon, 18 Mar 2019 22:10:24 -0400 Subject: [PATCH] Revert "New rule no-access-state-in-setstate (#190)" This reverts commit 987a95e87497f250e6d7f6dbaeb8abd12fa116cf. --- README.md | 17 --- src/rules/noAccessStateInSetstateRule.ts | 119 ------------------ .../no-access-state-in-setstate/test.tsx.lint | 65 ---------- .../no-access-state-in-setstate/tslint.json | 5 - 4 files changed, 206 deletions(-) delete mode 100644 src/rules/noAccessStateInSetstateRule.ts delete mode 100644 test/rules/no-access-state-in-setstate/test.tsx.lint delete mode 100644 test/rules/no-access-state-in-setstate/tslint.json diff --git a/README.md b/README.md index b7297a4..7f13511 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,6 @@ The built-in configuration preset you get with `"extends": "tslint-react"` is se size={size} /> ``` - - Rule options: _none_ - `jsx-ban-elements` (since v3.4.0) - Allows blacklisting of JSX elements with an optional explanatory message in the reported failure. - `jsx-ban-props` (since v2.3.0) @@ -122,22 +121,6 @@ The built-in configuration preset you get with `"extends": "tslint-react"` is se ); ``` - - Rule options: _none_ -- `no-access-state-in-setstate` - - Forbids accessing component state with `this.state` within `this.setState` - calls, since React might batch multiple `this.setState` calls, thus resulting - in accessing old state. Enforces use of callback argument instead. - ```ts - // bad - this.setState({ - counter: this.state.counter + 1 - }); - // good - this.setState( - prevState => ({ counter: prevState.counter + 1 }) - ); - ``` - - Rule options: _none_ ### Development diff --git a/src/rules/noAccessStateInSetstateRule.ts b/src/rules/noAccessStateInSetstateRule.ts deleted file mode 100644 index 8f197f7..0000000 --- a/src/rules/noAccessStateInSetstateRule.ts +++ /dev/null @@ -1,119 +0,0 @@ -/** - * @license - * Copyright 2018 Palantir Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import * as Lint from "tslint"; -import { isCallExpression, isClassDeclaration, isPropertyAccessExpression } from "tsutils"; -import * as ts from "typescript"; - -export class Rule extends Lint.Rules.AbstractRule { - /* tslint:disable:object-literal-sort-keys */ - public static metadata: Lint.IRuleMetadata = { - ruleName: "no-access-state-in-setstate", - description: "Reports usage of this.state within setState", - rationale: Lint.Utils.dedent` - Usage of this.state might result in errors when two state calls are - called in batch and thus referencing old state and not the current state. - See [setState()](https://reactjs.org/docs/react-component.html#setstate) in the React API reference. - `, - options: null, - optionsDescription: "", - type: "functionality", - typescriptOnly: false, - }; - /* tslint:enable:object-literal-sort-keys */ - - public static OBJECT_ARG_FAILURE = - "References to this.state are not allowed in the setState state change object."; - - public static CALLBACK_ARG_FAILURE = - "References to this.state are not allowed in the setState updater, use the callback arguments instead."; - - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithFunction(sourceFile, walk); - } -} - -function walk(ctx: Lint.WalkContext): void { - return ts.forEachChild(ctx.sourceFile, callbackForEachChild); - - function callbackForEachChild(node: ts.Node): void { - if (!isClassDeclaration(node)) { - return; - } - - ts.forEachChild(node, callbackForEachChildInClass); - } - - function callbackForEachChildInClass(node: ts.Node): void { - if (!isCallExpression(node)) { - return ts.forEachChild(node, callbackForEachChildInClass); - } - - const callExpressionArguments = node.arguments; - - if (!isPropertyAccessExpression(node.expression) || callExpressionArguments.length === 0) { - return; - } - - const propertyAccessExpression = node.expression; - - const isThisPropertyAccess = propertyAccessExpression.expression.kind === ts.SyntaxKind.ThisKeyword; - const isSetStateCall = propertyAccessExpression.name.text === "setState"; - - if (!isThisPropertyAccess || !isSetStateCall) { - return; - } - - const firstArgument = node.arguments[0]; - - if (ts.isObjectLiteralExpression(firstArgument)) { - ts.forEachChild(firstArgument, callbackForEachChildInSetStateObjectArgument); - } else if (ts.isArrowFunction(firstArgument) || ts.isFunctionExpression(firstArgument)) { - ts.forEachChild(firstArgument, callbackForEachChildInSetStateCallbackArgument); - } - } - - function callbackForEachChildInSetStateObjectArgument(node: ts.Node): void { - if (!isPropertyAccessExpression(node) || !isPropertyAccessExpression(node.expression)) { - return ts.forEachChild(node, callbackForEachChildInSetStateObjectArgument); - } - - if ( - node.expression.expression.kind !== ts.SyntaxKind.ThisKeyword || - node.expression.name.text !== "state" - ) { - return ts.forEachChild(node, callbackForEachChildInSetStateObjectArgument); - } - - ctx.addFailureAtNode(node, Rule.OBJECT_ARG_FAILURE); - } - - function callbackForEachChildInSetStateCallbackArgument(node: ts.Node): void { - if (!isPropertyAccessExpression(node) || !isPropertyAccessExpression(node.expression)) { - return ts.forEachChild(node, callbackForEachChildInSetStateCallbackArgument); - } - - if ( - node.expression.expression.kind !== ts.SyntaxKind.ThisKeyword || - node.expression.name.text !== "state" - ) { - return ts.forEachChild(node, callbackForEachChildInSetStateCallbackArgument); - } - - ctx.addFailureAtNode(node, Rule.CALLBACK_ARG_FAILURE); - } -} diff --git a/test/rules/no-access-state-in-setstate/test.tsx.lint b/test/rules/no-access-state-in-setstate/test.tsx.lint deleted file mode 100644 index 008ecf8..0000000 --- a/test/rules/no-access-state-in-setstate/test.tsx.lint +++ /dev/null @@ -1,65 +0,0 @@ -class SomeReactComponent extends React.Component { - - someClassFunction() { - - this.fooBar({ - foo: this.state.foo - }); - - this.setState({ - foo: "foo", - bar: this.barz - }); - - this.setState( - { - foo: "foo" - }, - () => this.fooBar(this.state.foo); - ); - - this.setState(prevState => ({ - foo: !prevState.foo - })); - - this.setState((prevState, currentProps) => ({ - foo: !prevState.foo, - bar: currentProps.bar - })); - - this.setState({ - foo: window.history.length - }); - - this.setState({ - foo: !this.props.bar - }); - - this.setState({ - foo: !this.state.foo - ~~~~~~~~~~~~~~ [0] - }); - - this.setState({ - foo: this.fooBar(this.state.foo) - ~~~~~~~~~~~~~~ [0] - }); - - this.setState((prevState, currentProps) => ({ - foo: !this.state.foo, - ~~~~~~~~~~~~~~ [1] - bar: currentProps.bar - })); - - this.setState((prevState, currentProps) => { - this.fooBar(this.state.foo); - ~~~~~~~~~~~~~~ [1] - return { - bar: !prevState.bar - }; - }); - } -} - -[0]: References to this.state are not allowed in the setState state change object. -[1]: References to this.state are not allowed in the setState updater, use the callback arguments instead. diff --git a/test/rules/no-access-state-in-setstate/tslint.json b/test/rules/no-access-state-in-setstate/tslint.json deleted file mode 100644 index 666f783..0000000 --- a/test/rules/no-access-state-in-setstate/tslint.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "rules": { - "no-access-state-in-setstate": true - } -}