Skip to content

Commit

Permalink
Add PureComponent support to prefer-stateless-function
Browse files Browse the repository at this point in the history
  • Loading branch information
teameh authored and yannickcr committed Aug 23, 2016
1 parent d4dee48 commit 7024678
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 31 deletions.
51 changes: 44 additions & 7 deletions docs/rules/prefer-stateless-function.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,33 @@ This rule will check your class based React components for

* methods/properties other than `displayName`, `propTypes`, `render` and useless constructor (same detection as ESLint [no-useless-constructor rule](http://eslint.org/docs/rules/no-useless-constructor))
* instance property other than `this.props` and `this.context`
* extension of `React.PureComponent` ()
* presence of `ref` attribute in JSX
* `render` method that return anything but JSX: `undefined`, `null`, etc. (only in React <15.0.0, see [shared settings](https://github.com/yannickcr/eslint-plugin-react/blob/master/README.md#configuration) for React version configuration)

If none of these 4 elements are found, the rule will warn you to write this component as a pure function.
If none of these elements are found, the rule will warn you to write this component as a pure function.

The following pattern is considered warnings:
The following pattern is considered a warning:

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

The following pattern is not considered warnings:
The following pattern is not considered a warning:

```js
```jsx
const Foo = function(props) {
return <div>{props.foo}</div>;
};
```

The following pattern is not considered warning in React <15.0.0:
The following pattern is not considered a warning in React <15.0.0:

```js
```jsx
class Foo extends React.Component {
render() {
if (!this.props.foo) {
Expand All @@ -43,3 +44,39 @@ class Foo extends React.Component {
}
}
```


## Rule Options

```js
...
"prefer-stateless-function": [<enabled>, { "ignorePureComponent": <ignorePureComponent> }]
...
```

* `enabled`: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0.
* `ignorePureComponent`: optional boolean set to `true` to ignore components extending from `React.PureComponent` (default to `false`).

### `ignorePureComponent`

When `true` the rule will ignore Components extending from `React.PureComponent` that use `this.props` or `this.context`.

The following patterns is considered okay and does not cause warnings:

```jsx
class Foo extends React.PureComponent {
render() {
return <div>{this.props.foo}</div>;
}
}
```

The following pattern is considered a warning because it's not using props or context:

```jsx
class Foo extends React.PureComponent {
render() {
return <div>Bar</div>;
}
}
```
2 changes: 1 addition & 1 deletion lib/rules/no-multi-comp.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ module.exports = {
* @returns {Boolean} True if the component is ignored, false if not.
*/
function isIgnored(component) {
return ignoreStateless === true && /Function/.test(component.node.type);
return ignoreStateless && /Function/.test(component.node.type);
}

// --------------------------------------------------------------------------
Expand Down
51 changes: 48 additions & 3 deletions lib/rules/prefer-stateless-function.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,23 @@ module.exports = {
category: 'Stylistic Issues',
recommended: false
},
schema: []
schema: [{
type: 'object',
properties: {
ignorePureComponents: {
default: false,
type: 'boolean'
}
},
additionalProperties: false
}]
},

create: Components.detect(function(context, components, utils) {

var configuration = context.options[0] || {};
var ignorePureComponents = configuration.ignorePureComponents || false;

var sourceCode = context.getSourceCode();

// --------------------------------------------------------------------------
Expand Down Expand Up @@ -213,6 +225,16 @@ module.exports = {
});
}

/**
* Mark component as pure as declared
* @param {ASTNode} node The AST node being checked.
*/
var markSCUAsDeclared = function (node) {
components.set(node, {
hasSCU: true
});
};

/**
* Mark a setState as used
* @param {ASTNode} node The AST node being checked.
Expand All @@ -223,6 +245,16 @@ module.exports = {
});
}

/**
* Mark a props or context as used
* @param {ASTNode} node The AST node being checked.
*/
function markPropsOrContextAsUsed(node) {
components.set(node, {
usePropsOrContext: true
});
}

/**
* Mark a ref as used
* @param {ASTNode} node The AST node being checked.
Expand All @@ -244,6 +276,12 @@ module.exports = {
}

return {
ClassDeclaration: function (node) {
if (ignorePureComponents && utils.isPureComponent(node)) {
markSCUAsDeclared(node);
}
},

// Mark `this` destructuring as a usage of `this`
VariableDeclarator: function(node) {
// Ignore destructuring on other than `this`
Expand All @@ -256,6 +294,7 @@ module.exports = {
return name !== 'props' && name !== 'context';
});
if (!useThis) {
markPropsOrContextAsUsed(node);
return;
}
markThisAsUsed(node);
Expand All @@ -264,11 +303,13 @@ module.exports = {
// Mark `this` usage
MemberExpression: function(node) {
// Ignore calls to `this.props` and `this.context`
if (
node.object.type !== 'ThisExpression' ||
if (node.object.type !== 'ThisExpression') {
return;
} else if (
(node.property.name || node.property.value) === 'props' ||
(node.property.name || node.property.value) === 'context'
) {
markPropsOrContextAsUsed(node);
return;
}
markThisAsUsed(node);
Expand Down Expand Up @@ -322,6 +363,10 @@ module.exports = {
continue;
}

if (list[component].hasSCU && list[component].usePropsOrContext) {
continue;
}

context.report({
node: list[component].node,
message: 'Component should be written as a pure function'
Expand Down
22 changes: 2 additions & 20 deletions lib/rules/require-optimization.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
'use strict';

var Components = require('../util/Components');
var pragmaUtil = require('../util/pragma');

module.exports = {
meta: {
Expand All @@ -29,15 +28,11 @@ module.exports = {
}]
},

create: Components.detect(function (context, components) {
create: Components.detect(function (context, components, utils) {
var MISSING_MESSAGE = 'Component is not optimized. Please add a shouldComponentUpdate method.';
var configuration = context.options[0] || {};
var allowDecorators = configuration.allowDecorators || [];

var pragma = pragmaUtil.getFromContext(context);
var pureComponentRegExp = new RegExp('^(' + pragma + '\\.)?PureComponent$');
var sourceCode = context.getSourceCode();

/**
* Checks to see if our component is decorated by PureRenderMixin via reactMixin
* @param {ASTNode} node The AST node being checked.
Expand Down Expand Up @@ -89,19 +84,6 @@ module.exports = {
return false;
};

/**
* Checks to see if our component extends React.PureComponent
* @param {ASTNode} node The AST node being checked.
* @returns {Boolean} True if node extends React.PureComponent, false if not.
*/
var isPureComponent = function (node) {
if (node.superClass) {
return pureComponentRegExp.test(sourceCode.getText(node.superClass));
}

return false;
};

/**
* Checks if we are declaring a shouldComponentUpdate method
* @param {ASTNode} node The AST node being checked.
Expand Down Expand Up @@ -186,7 +168,7 @@ module.exports = {
},

ClassDeclaration: function (node) {
if (!(hasPureRenderDecorator(node) || hasCustomDecorator(node) || isPureComponent(node))) {
if (!(hasPureRenderDecorator(node) || hasCustomDecorator(node) || utils.isPureComponent(node))) {
return;
}
markSCUAsDeclared(node);
Expand Down
13 changes: 13 additions & 0 deletions lib/util/Components.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,19 @@ function componentRule(rule, context) {
return relevantTags.length > 0;
},

/**
* Checks to see if our component extends React.PureComponent
*
* @param {ASTNode} node The AST node being checked.
* @returns {Boolean} True if node extends React.PureComponent, false if not
*/
isPureComponent: function (node) {
if (node.superClass) {
return new RegExp('^(' + pragma + '\\.)?PureComponent$').test(sourceCode.getText(node.superClass));
}
return false;
},

/**
* Check if the node is returning JSX
*
Expand Down
55 changes: 55 additions & 0 deletions tests/lib/rules/prefer-stateless-function.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,32 @@ ruleTester.run('prefer-stateless-function', rule, {
// Already a stateless (arrow) function
code: 'const Foo = ({foo}) => <div>{foo}</div>;',
parserOptions: parserOptions
}, {
// Extends from PureComponent and uses props
code: [
'class Foo extends React.PureComponent {',
' render() {',
' return <div>{this.props.foo}</div>;',
' }',
'}'
].join('\n'),
parserOptions: parserOptions,
options: [{
ignorePureComponents: true
}]
}, {
// Extends from PureComponent and uses context
code: [
'class Foo extends React.PureComponent {',
' render() {',
' return <div>{this.context.foo}</div>;',
' }',
'}'
].join('\n'),
parserOptions: parserOptions,
options: [{
ignorePureComponents: true
}]
}, {
// Has a lifecyle method
code: [
Expand Down Expand Up @@ -259,6 +285,35 @@ ruleTester.run('prefer-stateless-function', rule, {
errors: [{
message: 'Component should be written as a pure function'
}]
}, {
// Only extend PureComponent without use of props or context
code: [
'class Foo extends React.PureComponent {',
' render() {',
' return <div>foo</div>;',
' }',
'}'
].join('\n'),
parserOptions: parserOptions,
options: [{
ignorePureComponents: true
}],
errors: [{
message: 'Component should be written as a pure function'
}]
}, {
// Extends from PureComponent but no ignorePureComponents option
code: [
'class Foo extends React.PureComponent {',
' render() {',
' return <div>{this.props.foo}</div>;',
' }',
'}'
].join('\n'),
parserOptions: parserOptions,
errors: [{
message: 'Component should be written as a pure function'
}]
}, {
// Has only displayName (method) and render
code: [
Expand Down

0 comments on commit 7024678

Please sign in to comment.