Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion docs/rules/no-did-mount-set-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ var Hello = React.createClass({

```js
...
"no-did-mount-set-state": [<enabled>, <mode>]
"no-did-mount-set-state": [<enabled>, <modeInFunctions>, <modeViaMethods>]
...
```

Expand Down Expand Up @@ -90,3 +90,38 @@ var Hello = React.createClass({
}
});
```

### `disallow-via-methods` mode

By default this rule ignores any call to `this.setState` via methods called in `componentDidMount`. The `disallow-via-methods` mode makes this rule more strict by disallowing calls to `this.setState` even within methods.

The following patterns are considered warnings:

```js
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: these should be jsx

var Hello = React.createClass({
componentDidMount: function() {
this.setState({
name: this.props.name.toUpperCase()
});
},
render: function() {
return <div>Hello {this.state.name}</div>;
}
});
```

```js
var Hello = React.createClass({
componentDidMount: function() {
this.doSomethingToState();
},
doSomethingToState: function() {
this.setState({
name: newName
});
},
render: function() {
return <div>Hello {this.state.name}</div>;
}
});
```
37 changes: 36 additions & 1 deletion docs/rules/no-did-update-set-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ var Hello = React.createClass({

```js
...
"no-did-update-set-state": [<enabled>, <mode>]
"no-did-update-set-state": [<enabled>, <modeInFunctions>, <modeViaMethods>]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a good idea to use an options object for these modes instead of positional arguments.

That would be a breaking change, so perhaps it should be done in a followup PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both could be supported with eslint schemas as semver-minor - ie, a "mode" string, and then otherwise, an options object.

...
```

Expand Down Expand Up @@ -88,3 +88,38 @@ var Hello = React.createClass({
}
});
```

### `disallow-via-methods` mode

By default this rule ignores any call to `this.setState` via methods called in `componentDidUpdate`. The `disallow-via-methods` mode makes this rule more strict by disallowing calls to `this.setState` even within methods.

The following patterns are considered warnings:

```js
var Hello = React.createClass({
componentDidUpdate: function() {
this.setState({
name: this.props.name.toUpperCase()
});
},
render: function() {
return <div>Hello {this.state.name}</div>;
}
});
```

```js
var Hello = React.createClass({
componentDidUpdate: function() {
this.doSomethingToState();
},
doSomethingToState: function() {
this.setState({
name: newName
});
},
render: function() {
return <div>Hello {this.state.name}</div>;
}
});
```
26 changes: 19 additions & 7 deletions lib/rules/no-did-mount-set-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
*/
'use strict';

var Components = require('../util/Components');
var noSetStateViaMethods = require('../util/no-set-state-via-methods');

var DISALLOW_IN_FUNCTIONS = 'disallow-in-func';
var ALLOW_IN_FUNCTIONS = 'allow-in-func';
var DISALLOW_VIA_METHODS = 'disallow-via-methods';
var ALLOW_VIA_METHODS = 'allow-via-methods';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
Expand All @@ -17,20 +25,21 @@ module.exports = {
},

schema: [{
enum: ['disallow-in-func']
enum: [DISALLOW_IN_FUNCTIONS, ALLOW_IN_FUNCTIONS]
}, {
enum: [DISALLOW_VIA_METHODS, ALLOW_VIA_METHODS]
}]
},

create: function(context) {

var mode = context.options[0] || 'allow-in-func';
create: Components.detect(function(context, components, utils) {
var modeInFunctions = context.options[0] || ALLOW_IN_FUNCTIONS;
var modeViaMethods = context.options[1] || ALLOW_VIA_METHODS;

// --------------------------------------------------------------------------
// Public
// --------------------------------------------------------------------------

return {

CallExpression: function(node) {
var callee = node.callee;
if (
Expand All @@ -49,7 +58,7 @@ module.exports = {
if (
(ancestors[i].type !== 'Property' && ancestors[i].type !== 'MethodDefinition') ||
ancestors[i].key.name !== 'componentDidMount' ||
(mode !== 'disallow-in-func' && depth > 1)
(modeInFunctions !== DISALLOW_IN_FUNCTIONS && depth > 1)
) {
continue;
}
Expand All @@ -59,8 +68,11 @@ module.exports = {
});
break;
}
if (modeViaMethods === DISALLOW_VIA_METHODS) {
noSetStateViaMethods.run('componentDidMount', callee, ancestors, context, components, utils);
}
}
};

}
})
};
27 changes: 19 additions & 8 deletions lib/rules/no-did-update-set-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
*/
'use strict';

var Components = require('../util/Components');
var noSetStateViaMethods = require('../util/no-set-state-via-methods');

var DISALLOW_IN_FUNCTIONS = 'disallow-in-func';
var ALLOW_IN_FUNCTIONS = 'allow-in-func';
var DISALLOW_VIA_METHODS = 'disallow-via-methods';
var ALLOW_VIA_METHODS = 'allow-via-methods';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
Expand All @@ -17,20 +25,21 @@ module.exports = {
},

schema: [{
enum: ['disallow-in-func']
enum: [DISALLOW_IN_FUNCTIONS, ALLOW_IN_FUNCTIONS]
}, {
enum: [DISALLOW_VIA_METHODS, ALLOW_VIA_METHODS]
}]
},

create: function(context) {

var mode = context.options[0] || 'allow-in-func';
create: Components.detect(function(context, components, utils) {
var modeInFunctions = context.options[0] || ALLOW_IN_FUNCTIONS;
var modeViaMethods = context.options[1] || ALLOW_VIA_METHODS;

// --------------------------------------------------------------------------
// Public
// --------------------------------------------------------------------------

return {

CallExpression: function(node) {
var callee = node.callee;
if (
Expand All @@ -49,7 +58,7 @@ module.exports = {
if (
(ancestors[i].type !== 'Property' && ancestors[i].type !== 'MethodDefinition') ||
ancestors[i].key.name !== 'componentDidUpdate' ||
(mode !== 'disallow-in-func' && depth > 1)
(modeInFunctions !== DISALLOW_IN_FUNCTIONS && depth > 1)
) {
continue;
}
Expand All @@ -59,8 +68,10 @@ module.exports = {
});
break;
}
if (modeViaMethods === DISALLOW_VIA_METHODS) {
noSetStateViaMethods.run('componentDidUpdate', callee, ancestors, context, components, utils);
}
}
};

}
})
};
119 changes: 119 additions & 0 deletions lib/util/no-set-state-via-methods.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/**
* @fileoverview Prevent usage of findDOMNode
* @author Valentin Agachi
*/

'use strict';

function getCalleeName(node) {
if (!node.callee) {
return null;
}
var callee = node.callee;
if (
callee.type !== 'MemberExpression' ||
(
callee.object.type !== 'ThisExpression' &&
callee.object.type !== 'Identifier'
) ||
callee.property.type !== 'Identifier'
) {
return null;
}

var obj = callee.object.type === 'ThisExpression'
? 'this'
: callee.object.name;
var prop = callee.property.name;

return obj + '.' + prop;
}

function isMethodDefinition(node) {
return (
node.type === 'MethodDefinition' &&
node.kind === 'method' &&
!node.static
);
}
function isObjectPropertyMethod(node) {
return (
node.type === 'Property' &&
node.method
);
}

function functionExpressionCallsExpression(node, methodName) {
var verifyBodyNode = function(bodyNode) {
switch (bodyNode.type) {
case 'ExpressionStatement':
if (bodyNode.expression.type === 'CallExpression') {
return getCalleeName(bodyNode.expression) === 'this.' + methodName;
}
break;
case 'IfStatement':
return (
bodyNode.consequent.body.some(verifyBodyNode) ||
(bodyNode.alternate && bodyNode.alternate.body.some(verifyBodyNode))
);
default:
return false;
}
return false;
};

return node.value.body.body.some(verifyBodyNode);
}

function lifecycleCallsMethod(lifecycleMethod, methodName) {
return function(node) {
var isLifecycleMethod = (
(
isMethodDefinition(node) ||
isObjectPropertyMethod(node)
) &&
node.key.name === lifecycleMethod
);
return isLifecycleMethod &&
functionExpressionCallsExpression(node, methodName);
};
}

module.exports = {
run: function(lifecycleMethod, callee, ancestors, context, components, utils) {
var method;
var methodName;

var es6Class = utils.getParentES6Component();
if (es6Class) {
method = ancestors.find(isMethodDefinition);
methodName = method.key.name;
var classBody = es6Class.body.body;
// Loop through all the component's methods to find componentDidUpdate
// and search for call expressions of `methodName`
if (classBody.some(lifecycleCallsMethod(lifecycleMethod, methodName))) {
context.report({
node: callee,
message: 'Do not use `setState` in other methods called in `' + lifecycleMethod + '`'
});
}
return;
}

var es5Class = utils.getParentES5Component();
if (es5Class) {
method = ancestors.find(isObjectPropertyMethod);
methodName = method.key.name;
// Loop through all the component's methods to find componentDidUpdate
// and search for call expressions of `methodName`
var classProperties = es5Class.properties;
if (classProperties.some(lifecycleCallsMethod(lifecycleMethod, methodName))) {
context.report({
node: callee,
message: 'Do not use `setState` in other methods called in `' + lifecycleMethod + '`'
});
}
return;
}
}
};
Loading