diff --git a/docs/rules/no-direct-mutation-state.md b/docs/rules/no-direct-mutation-state.md index da0dbf5d72..3a8c8bdd05 100644 --- a/docs/rules/no-direct-mutation-state.md +++ b/docs/rules/no-direct-mutation-state.md @@ -3,6 +3,8 @@ NEVER mutate `this.state` directly, as calling `setState()` afterwards may replace the mutation you made. Treat `this.state` as if it were immutable. +The only place that's acceptable to assign this.state is in a ES6 `class` component constructor. + ## Rule Details This rule is aimed to forbid the use of mutating `this.state` directly. @@ -18,6 +20,17 @@ var Hello = createReactClass({ return
Hello {this.state.name}
; } }); + +class Hello extends React.Component { + constructor(props) { + super(props) + + // Assign at instance creation time, not on a callback + doSomethingAsync(() => { + this.state = 'bad'; + }); + } +} ``` @@ -34,4 +47,14 @@ var Hello = createReactClass({ return
Hello {this.state.name}
; } }); + +class Hello extends React.Component { + constructor(props) { + super(props) + + this.state = { + foo: 'bar', + } + } +} ``` diff --git a/lib/rules/no-direct-mutation-state.js b/lib/rules/no-direct-mutation-state.js index e24ce1b42a..fe71927003 100644 --- a/lib/rules/no-direct-mutation-state.js +++ b/lib/rules/no-direct-mutation-state.js @@ -1,10 +1,10 @@ /** * @fileoverview Prevent direct mutation of this.state * @author David Petersen + * @author Nicolas Fernandez <@burabure> */ 'use strict'; -var has = require('has'); var Components = require('../util/Components'); // ------------------------------------------------------------------------------ @@ -49,41 +49,62 @@ module.exports = { // -------------------------------------------------------------------------- // Public // -------------------------------------------------------------------------- + let inConstructor = false; + let inCallExpression = false; return { - AssignmentExpression: function(node) { - var item; - if (!node.left || !node.left.object || !node.left.object.object) { + MethodDefinition(node) { + if (node.kind === 'constructor') { + inConstructor = true; + } + }, + + CallExpression: function() { + inCallExpression = true; + }, + + AssignmentExpression(node) { + let item; + if ((inConstructor && !inCallExpression) || !node.left || !node.left.object) { return; } - item = node.left.object; - while (item.object.property) { + item = node.left; + while (item.object && item.object.property) { item = item.object; } if ( item.object.type === 'ThisExpression' && item.property.name === 'state' ) { - var component = components.get(utils.getParentComponent()); - var mutations = component && component.mutations || []; + const component = components.get(utils.getParentComponent()); + const mutations = (component && component.mutations) || []; mutations.push(node.left.object); components.set(node, { mutateSetState: true, - mutations: mutations + mutations }); } }, - 'Program:exit': function() { - var list = components.list(); - for (var component in list) { - if (!has(list, component) || isValid(list[component])) { - continue; - } - reportMutations(list[component]); + 'CallExpression:exit': function() { + inCallExpression = false; + }, + + 'MethodDefinition:exit': function (node) { + if (node.kind === 'constructor') { + inConstructor = false; } + }, + + 'Program:exit': function () { + const list = components.list(); + + Object.keys(list).forEach((key) => { + if (!isValid(list[key])) { + reportMutations(list[key]); + } + }); } }; - }) }; diff --git a/tests/lib/rules/no-direct-mutation-state.js b/tests/lib/rules/no-direct-mutation-state.js index 40cbeecdaa..515944c76e 100644 --- a/tests/lib/rules/no-direct-mutation-state.js +++ b/tests/lib/rules/no-direct-mutation-state.js @@ -61,6 +61,14 @@ ruleTester.run('no-direct-mutation-state', rule, { ' }', '}' ].join('\n') + }, { + code: [ + 'class Hello extends React.Component {', + ' constructor() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n') }], invalid: [{ @@ -118,6 +126,111 @@ ruleTester.run('no-direct-mutation-state', rule, { line: 4, column: 5 }] + }, { + code: [ + 'class Hello extends React.Component {', + ' constructor() {', + ' someFn()', + ' }', + ' someFn() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' constructor(props) {', + ' super(props)', + ' doSomethingAsync(() => {', + ' this.state = "bad";', + ' });', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillMount() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentDidMount() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillReceiveProps() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' shouldComponentUpdate() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillUpdate() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentDidUpdate() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' componentWillUnmount() {', + ' this.state.foo = "bar"', + ' }', + '}' + ].join('\n'), + errors: [{ + message: 'Do not mutate state directly. Use setState().' + }] } /** * Would be nice to prevent this too