Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable allow-in-func mode by default in no-did-mount-set-state and no-did-update-set-state rules #702

Merged
merged 2 commits into from
Jul 27, 2016
Merged

Enable allow-in-func mode by default in no-did-mount-set-state and no-did-update-set-state rules #702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53ed33b
Enable allow-in-func mode by default in no-did-mount-set-state
lencioni Jul 24, 2016
669d597
Enable allow-in-func mode by default in no-did-update-set-state
lencioni Jul 24, 2016
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
12 changes: 5 additions & 7 deletions docs/rules/no-did-mount-set-state.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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