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.

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

Already on GitHub? Sign in to your account

Add state-in-constructor rule #1945

Merged
merged 9 commits into from Jan 21, 2019
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -133,6 +133,7 @@ Enable the rules that you would like to use.
* [react/self-closing-comp](docs/rules/self-closing-comp.md): Prevent extra closing tags for components without children (fixable)
* [react/sort-comp](docs/rules/sort-comp.md): Enforce component methods order (fixable)
* [react/sort-prop-types](docs/rules/sort-prop-types.md): Enforce propTypes declarations alphabetical sorting
* [react/state-in-constructor](docs/rules/state-in-constructor.md): Enforce the state initialization style to be either in a constructor or with a class property
* [react/style-prop-object](docs/rules/style-prop-object.md): Enforce style prop value being an object
* [react/void-dom-elements-no-children](docs/rules/void-dom-elements-no-children.md): Prevent void DOM elements (e.g. `<img />`, `<br />`) from receiving children

Expand Down
74 changes: 74 additions & 0 deletions docs/rules/state-in-constructor.md
@@ -0,0 +1,74 @@
# Enforce state initialization style (react/state-in-constructor)

This rule will enforce the state initialization style to be either in a constructor or with a class property.

## Rule Options

```js
...
"react/state-in-constructor": [<enabled>, <mode>]
...
```

### `always` mode

Will enforce the state initialization style to be in a constructor. This is the default mode.

The following patterns are considered warnings:

```jsx
class Foo extends React.Component {
state = { bar: 0 }
render() {
return <div>Foo</div>
}
}
```

The following patterns are **not** considered warnings:

```jsx
class Foo extends React.Component {
constructor(props) {
super(props)
this.state = { bar: 0 }
}
render() {
return <div>Foo</div>
}
}
```

### `never` mode

Will enforce the state initialization style to be with a class property.

The following patterns are considered warnings:

```jsx
class Foo extends React.Component {
constructor(props) {
super(props)
this.state = { bar: 0 }
}
render() {
return <div>Foo</div>
}
}
```

The following patterns are **not** considered warnings:

```jsx
class Foo extends React.Component {
state = { bar: 0 }
render() {
return <div>Foo</div>
}
}
```


## When Not To Use It

When the way a component state is being initialized doesn't matter.
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -79,6 +79,7 @@ const allRules = {
'self-closing-comp': require('./lib/rules/self-closing-comp'),
'sort-comp': require('./lib/rules/sort-comp'),
'sort-prop-types': require('./lib/rules/sort-prop-types'),
'state-in-constructor': require('./lib/rules/state-in-constructor'),
'style-prop-object': require('./lib/rules/style-prop-object'),
'void-dom-elements-no-children': require('./lib/rules/void-dom-elements-no-children')
};
Expand Down
13 changes: 2 additions & 11 deletions lib/rules/no-direct-mutation-state.js
Expand Up @@ -59,15 +59,6 @@ module.exports = {
return node;
}

/**
* Determine if this MemberExpression is for `this.state`
* @param {Object} node The node to process
* @returns {Boolean}
*/
function isStateMemberExpression(node) {
return node.object.type === 'ThisExpression' && node.property.name === 'state';
}

/**
* Determine if we should currently ignore assignments in this component.
* @param {?Object} component The component to process
Expand Down Expand Up @@ -101,7 +92,7 @@ module.exports = {
return;
}
const item = getOuterMemberExpression(node.left);
if (isStateMemberExpression(item)) {
if (utils.isStateMemberExpression(item)) {
const mutations = (component && component.mutations) || [];
mutations.push(node.left.object);
components.set(node, {
Expand All @@ -117,7 +108,7 @@ module.exports = {
return;
}
const item = getOuterMemberExpression(node.argument);
if (isStateMemberExpression(item)) {
if (utils.isStateMemberExpression(item)) {
const mutations = (component && component.mutations) || [];
mutations.push(item);
components.set(node, {
Expand Down
58 changes: 58 additions & 0 deletions lib/rules/state-in-constructor.js
@@ -0,0 +1,58 @@
/**
* @fileoverview Enforce the state initialization style to be either in a constructor or with a class property
* @author Kanitkorn Sujautra
*/
'use strict';

const Components = require('../util/Components');
const docsUrl = require('../util/docsUrl');

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'State initialization in an ES6 class component should be in a constructor',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('state-in-constructor')
},
schema: [{
enum: ['always', 'never']
}]
},

create: Components.detect((context, components, utils) => {
const option = context.options[0] || 'always';
return {
ClassProperty: function (node) {
if (
option === 'always' &&
!node.static &&
node.key.name === 'state' &&
utils.getParentES6Component()
) {
context.report({
node,
message: 'State initialization should be in a constructor'
});
}
},
AssignmentExpression(node) {
if (
option === 'never' &&
utils.isStateMemberExpression(node.left) &&
utils.inConstructor() &&
utils.getParentES6Component()
) {
context.report({
node,
message: 'State initialization should be in a class property'
});
}
}
};
})
};
24 changes: 24 additions & 0 deletions lib/util/Components.js
Expand Up @@ -298,6 +298,30 @@ function componentRule(rule, context) {
return calledOnPragma;
},

/**
* Check if we are in a class constructor
* @return {boolean} true if we are in a class constructor, false if not
*/
inConstructor: function() {
let scope = context.getScope();
while (scope) {
if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') {
return true;
}
scope = scope.upper;
}
return false;
},

/**
* Determine if the node is MemberExpression of `this.state`
* @param {Object} node The node to process
* @returns {Boolean}
*/
isStateMemberExpression: function(node) {
return node.type === 'MemberExpression' && node.object.type === 'ThisExpression' && node.property.name === 'state';
},

getReturnPropertyAndNode(ASTnode) {
let property;
let node = ASTnode;
Expand Down
19 changes: 2 additions & 17 deletions lib/util/usedPropTypes.js
Expand Up @@ -200,21 +200,6 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
return key.type === 'Identifier' ? key.name : key.value;
}

/**
* Check if we are in a class constructor
* @return {boolean} true if we are in a class constructor, false if not
*/
function inConstructor() {
let scope = context.getScope();
while (scope) {
if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') {
return true;
}
scope = scope.upper;
}
return false;
}

/**
* Retrieve the name of a property node
* @param {ASTNode} node The AST node with the property.
Expand All @@ -226,7 +211,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
const isDirectPrevProp = DIRECT_PREV_PROPS_REGEX.test(sourceCode.getText(node));
const isDirectSetStateProp = isPropArgumentInSetStateUpdater(node);
const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component();
const isNotInConstructor = !inConstructor(node);
const isNotInConstructor = !utils.inConstructor(node);
const isNotInLifeCycleMethod = !inLifeCycleMethod();
const isNotInSetStateUpdater = !inSetStateUpdater();
if ((isDirectProp || isDirectNextProp || isDirectPrevProp || isDirectSetStateProp)
Expand Down Expand Up @@ -383,7 +368,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
|| DIRECT_NEXT_PROPS_REGEX.test(nodeSource)
|| DIRECT_PREV_PROPS_REGEX.test(nodeSource);
const reportedNode = (
!isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ?
!isDirectProp && !utils.inConstructor() && !inComponentWillReceiveProps() ?
node.parent.property :
node.property
);
Expand Down