diff --git a/docs/rules/prefer-stateless-function.md b/docs/rules/prefer-stateless-function.md index 5bedda460c..b42c3c0ca7 100644 --- a/docs/rules/prefer-stateless-function.md +++ b/docs/rules/prefer-stateless-function.md @@ -8,14 +8,15 @@ 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
Hello {this.props.name}
; @@ -23,17 +24,17 @@ var Hello = React.createClass({ }); ``` -The following pattern is not considered warnings: +The following pattern is not considered a warning: -```js +```jsx const Foo = function(props) { return
{props.foo}
; }; ``` -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) { @@ -43,3 +44,39 @@ class Foo extends React.Component { } } ``` + + +## Rule Options + +```js +... +"prefer-stateless-function": [, { "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
{this.props.foo}
; + } +} +``` + +The following pattern is considered a warning because it's not using props or context: + +```jsx +class Foo extends React.PureComponent { + render() { + return
Bar
; + } +} +``` diff --git a/lib/rules/no-multi-comp.js b/lib/rules/no-multi-comp.js index 25c160f1ec..2f165fe32c 100644 --- a/lib/rules/no-multi-comp.js +++ b/lib/rules/no-multi-comp.js @@ -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); } // -------------------------------------------------------------------------- diff --git a/lib/rules/prefer-stateless-function.js b/lib/rules/prefer-stateless-function.js index c05b9b0716..e4f2ecc9a4 100644 --- a/lib/rules/prefer-stateless-function.js +++ b/lib/rules/prefer-stateless-function.js @@ -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(); // -------------------------------------------------------------------------- @@ -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. @@ -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. @@ -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` @@ -256,6 +294,7 @@ module.exports = { return name !== 'props' && name !== 'context'; }); if (!useThis) { + markPropsOrContextAsUsed(node); return; } markThisAsUsed(node); @@ -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); @@ -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' diff --git a/lib/rules/require-optimization.js b/lib/rules/require-optimization.js index afb80ca061..e843e278ff 100644 --- a/lib/rules/require-optimization.js +++ b/lib/rules/require-optimization.js @@ -5,7 +5,6 @@ 'use strict'; var Components = require('../util/Components'); -var pragmaUtil = require('../util/pragma'); module.exports = { meta: { @@ -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. @@ -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. @@ -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); diff --git a/lib/util/Components.js b/lib/util/Components.js index 80f4a1e871..012e9c2c92 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -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 * diff --git a/tests/lib/rules/prefer-stateless-function.js b/tests/lib/rules/prefer-stateless-function.js index 38981efcfc..d2f945b50f 100644 --- a/tests/lib/rules/prefer-stateless-function.js +++ b/tests/lib/rules/prefer-stateless-function.js @@ -38,6 +38,32 @@ ruleTester.run('prefer-stateless-function', rule, { // Already a stateless (arrow) function code: 'const Foo = ({foo}) =>
{foo}
;', parserOptions: parserOptions + }, { + // Extends from PureComponent and uses props + code: [ + 'class Foo extends React.PureComponent {', + ' render() {', + ' return
{this.props.foo}
;', + ' }', + '}' + ].join('\n'), + parserOptions: parserOptions, + options: [{ + ignorePureComponents: true + }] + }, { + // Extends from PureComponent and uses context + code: [ + 'class Foo extends React.PureComponent {', + ' render() {', + ' return
{this.context.foo}
;', + ' }', + '}' + ].join('\n'), + parserOptions: parserOptions, + options: [{ + ignorePureComponents: true + }] }, { // Has a lifecyle method code: [ @@ -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
foo
;', + ' }', + '}' + ].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
{this.props.foo}
;', + ' }', + '}' + ].join('\n'), + parserOptions: parserOptions, + errors: [{ + message: 'Component should be written as a pure function' + }] }, { // Has only displayName (method) and render code: [