Skip to content

Commit

Permalink
Enable allow-in-func mode by default in no-did-mount-set-state and no…
Browse files Browse the repository at this point in the history
…-did-update-set-state rules (fixes #686)
  • Loading branch information
lencioni authored and yannickcr committed Jul 27, 2016
1 parent 2105cbc commit 376dc53
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 24 deletions.
12 changes: 5 additions & 7 deletions docs/rules/no-did-mount-set-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Updating the state after a component mount will trigger a second `render()` call

## Rule Details

This rule is aimed to forbid the use of `this.setState` in `componentDidMount`.
This rule is aimed to forbid the use of `this.setState` in `componentDidMount` outside of functions, such as callbacks.

The following patterns are considered warnings:

Expand All @@ -21,6 +21,8 @@ var Hello = React.createClass({
});
```

The following patterns are not considered warnings:

```js
var Hello = React.createClass({
componentDidMount: function() {
Expand All @@ -36,8 +38,6 @@ var Hello = React.createClass({
});
```

The following patterns are not considered warnings:

```js
var Hello = React.createClass({
componentDidMount: function() {
Expand All @@ -57,9 +57,9 @@ var Hello = React.createClass({
...
```

### `allow-in-func` mode
### `disallow-in-func` mode

By default this rule forbids any call to `this.setState` in `componentDidMount`. But since `componentDidMount` is a common place to set some event listeners, you may end up with calls to `this.setState` in some callbacks. The `allow-in-func` mode allows you to use `this.setState` in `componentDidMount` as long as they are called within a function.
By default this rule forbids any call to `this.setState` in `componentDidMount` outside of functions. The `disallow-in-func` mode makes this rule more strict by disallowing calls to `this.setState` even within functions.

The following patterns are considered warnings:

Expand All @@ -76,8 +76,6 @@ var Hello = React.createClass({
});
```

The following patterns are not considered warnings:

```js
var Hello = React.createClass({
componentDidMount: function() {
Expand Down
19 changes: 16 additions & 3 deletions docs/rules/no-did-update-set-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@ var Hello = React.createClass({
});
```

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

## Rule Options

```js
Expand All @@ -42,7 +57,7 @@ var Hello = React.createClass({

### `allow-in-func` mode

By default this rule forbids any call to `this.setState` in `componentDidUpdate`. But in certain cases you may need to perform asynchronous calls in `componentDidUpdate` that may end up with calls to `this.setState`. The `allow-in-func` mode allows you to use `this.setState` in `componentDidUpdate` as long as they are called within a function.
By default this rule forbids any call to `this.setState` in `componentDidUpdate` outside of functions. The `disallow-in-func` mode makes this rule more strict by disallowing calls to `this.setState` even within functions.

The following patterns are considered warnings:

Expand All @@ -59,8 +74,6 @@ var Hello = React.createClass({
});
```

The following patterns are not considered warnings:

```js
var Hello = React.createClass({
componentDidUpdate: function() {
Expand Down
6 changes: 3 additions & 3 deletions lib/rules/no-did-mount-set-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

module.exports = function(context) {

var mode = context.options[0] || 'never';
var mode = context.options[0] || 'allow-in-func';

// --------------------------------------------------------------------------
// Public
Expand All @@ -36,7 +36,7 @@ module.exports = function(context) {
if (
(ancestors[i].type !== 'Property' && ancestors[i].type !== 'MethodDefinition') ||
ancestors[i].key.name !== 'componentDidMount' ||
(mode === 'allow-in-func' && depth > 1)
(mode !== 'disallow-in-func' && depth > 1)
) {
continue;
}
Expand All @@ -52,5 +52,5 @@ module.exports = function(context) {
};

module.exports.schema = [{
enum: ['allow-in-func']
enum: ['disallow-in-func']
}];
6 changes: 3 additions & 3 deletions lib/rules/no-did-update-set-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

module.exports = function(context) {

var mode = context.options[0] || 'never';
var mode = context.options[0] || 'allow-in-func';

// --------------------------------------------------------------------------
// Public
Expand All @@ -36,7 +36,7 @@ module.exports = function(context) {
if (
(ancestors[i].type !== 'Property' && ancestors[i].type !== 'MethodDefinition') ||
ancestors[i].key.name !== 'componentDidUpdate' ||
(mode === 'allow-in-func' && depth > 1)
(mode !== 'disallow-in-func' && depth > 1)
) {
continue;
}
Expand All @@ -52,5 +52,5 @@ module.exports = function(context) {
};

module.exports.schema = [{
enum: ['allow-in-func']
enum: ['disallow-in-func']
}];
10 changes: 6 additions & 4 deletions tests/lib/rules/no-did-mount-set-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ ruleTester.run('no-did-mount-set-state', rule, {
' }',
'});'
].join('\n'),
options: ['allow-in-func'],
parserOptions: parserOptions
}, {
code: [
Expand All @@ -81,7 +80,6 @@ ruleTester.run('no-did-mount-set-state', rule, {
'});'
].join('\n'),
parser: 'babel-eslint',
options: ['allow-in-func'],
parserOptions: parserOptions
}],

Expand Down Expand Up @@ -123,7 +121,7 @@ ruleTester.run('no-did-mount-set-state', rule, {
' }',
'});'
].join('\n'),
options: ['allow-in-func'],
options: ['disallow-in-func'],
parserOptions: parserOptions,
errors: [{
message: 'Do not use setState in componentDidMount'
Expand All @@ -139,7 +137,7 @@ ruleTester.run('no-did-mount-set-state', rule, {
'}'
].join('\n'),
parser: 'babel-eslint',
options: ['allow-in-func'],
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidMount'
}]
Expand All @@ -156,6 +154,7 @@ ruleTester.run('no-did-mount-set-state', rule, {
'});'
].join('\n'),
parserOptions: parserOptions,
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidMount'
}]
Expand All @@ -172,6 +171,7 @@ ruleTester.run('no-did-mount-set-state', rule, {
'}'
].join('\n'),
parser: 'babel-eslint',
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidMount'
}]
Expand Down Expand Up @@ -217,6 +217,7 @@ ruleTester.run('no-did-mount-set-state', rule, {
].join('\n'),
parser: 'babel-eslint',
parserOptions: parserOptions,
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidMount'
}]
Expand All @@ -229,6 +230,7 @@ ruleTester.run('no-did-mount-set-state', rule, {
'}'
].join('\n'),
parser: 'babel-eslint',
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidMount'
}]
Expand Down
10 changes: 6 additions & 4 deletions tests/lib/rules/no-did-update-set-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ ruleTester.run('no-did-update-set-state', rule, {
' }',
'});'
].join('\n'),
options: ['allow-in-func'],
parserOptions: parserOptions
}, {
code: [
Expand All @@ -81,7 +80,6 @@ ruleTester.run('no-did-update-set-state', rule, {
'});'
].join('\n'),
parser: 'babel-eslint',
options: ['allow-in-func'],
parserOptions: parserOptions
}],

Expand Down Expand Up @@ -123,7 +121,7 @@ ruleTester.run('no-did-update-set-state', rule, {
' }',
'});'
].join('\n'),
options: ['allow-in-func'],
options: ['disallow-in-func'],
parserOptions: parserOptions,
errors: [{
message: 'Do not use setState in componentDidUpdate'
Expand All @@ -139,7 +137,7 @@ ruleTester.run('no-did-update-set-state', rule, {
'}'
].join('\n'),
parser: 'babel-eslint',
options: ['allow-in-func'],
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidUpdate'
}]
Expand All @@ -156,6 +154,7 @@ ruleTester.run('no-did-update-set-state', rule, {
'});'
].join('\n'),
parserOptions: parserOptions,
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidUpdate'
}]
Expand All @@ -172,6 +171,7 @@ ruleTester.run('no-did-update-set-state', rule, {
'}'
].join('\n'),
parser: 'babel-eslint',
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidUpdate'
}]
Expand Down Expand Up @@ -217,6 +217,7 @@ ruleTester.run('no-did-update-set-state', rule, {
].join('\n'),
parser: 'babel-eslint',
parserOptions: parserOptions,
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidUpdate'
}]
Expand All @@ -229,6 +230,7 @@ ruleTester.run('no-did-update-set-state', rule, {
'}'
].join('\n'),
parser: 'babel-eslint',
options: ['disallow-in-func'],
errors: [{
message: 'Do not use setState in componentDidUpdate'
}]
Expand Down

0 comments on commit 376dc53

Please sign in to comment.