diff --git a/README.md b/README.md index 06f97b7b75..a83bc0887c 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](# * [react/no-unescaped-entities](docs/rules/no-unescaped-entities.md): Prevent invalid characters from appearing in markup * [react/no-unknown-property](docs/rules/no-unknown-property.md): Prevent usage of unknown DOM property (fixable) * [react/no-unused-prop-types](docs/rules/no-unused-prop-types.md): Prevent definitions of unused prop types +* [react/no-useless-scu](docs/rules/no-useless-scu.md): Prevent usage of `shouldComponentUpdate` when extending React.PureComponent * [react/prefer-es6-class](docs/rules/prefer-es6-class.md): Enforce ES5 or ES6 class for React Components * [react/prefer-stateless-function](docs/rules/prefer-stateless-function.md): Enforce stateless React Components to be written as a pure function * [react/prop-types](docs/rules/prop-types.md): Prevent missing props validation in a React component definition diff --git a/docs/rules/no-useless-scu.md b/docs/rules/no-useless-scu.md new file mode 100644 index 0000000000..2ebded423f --- /dev/null +++ b/docs/rules/no-useless-scu.md @@ -0,0 +1,64 @@ +# Prevent usage of shouldComponentUpdate when extending React.PureComponent (react/no-useless-scu) + +Warns if you have `shouldComponentUpdate` defined when defining a component that extends React.PureComponent. +While having `shouldComponentUpdate` will still work, it becomes pointless to extend PureComponent. + +## Rule Details + +The following patterns are considered warnings: + +```jsx +class Foo extends React.PureComponent { + shouldComponentUpdate() { + return true; + } + + render() { + return
Radical!
+ } +} + +function Bar() { + return class Baz extends React.PureComponent { + shouldComponentUpdate() { + return true; + } + + render() { + return
Groovy!
+ } + } +} +``` + +The following patterns are not considered warnings: + +```jsx +class Foo extends React.Component { + shouldComponentUpdate() { + return true; + } + + render() { + return
Radical!
+ } +} + +function Bar() { + return class Baz extends React.Component { + shouldComponentUpdate() { + return true; + } + + render() { + return
Groovy!
+ } + } +} + +class Qux extends React.PureComponent { + render() { + return
Tubular!
+ } +} +``` diff --git a/index.js b/index.js index 914e4433f3..cf3f4e1868 100644 --- a/index.js +++ b/index.js @@ -63,7 +63,8 @@ var allRules = { 'no-comment-textnodes': require('./lib/rules/no-comment-textnodes'), 'require-extension': require('./lib/rules/require-extension'), 'wrap-multilines': require('./lib/rules/wrap-multilines'), - 'jsx-tag-spacing': require('./lib/rules/jsx-tag-spacing') + 'jsx-tag-spacing': require('./lib/rules/jsx-tag-spacing'), + 'no-useless-scu': require('./lib/rules/no-useless-scu') }; function filterRules(rules, predicate) { diff --git a/lib/rules/no-useless-scu.js b/lib/rules/no-useless-scu.js new file mode 100644 index 0000000000..834ef3d2be --- /dev/null +++ b/lib/rules/no-useless-scu.js @@ -0,0 +1,94 @@ +/** + * @fileoverview Flag shouldComponentUpdate when extending PureComponent + */ +'use strict'; + +var Components = require('../util/Components'); + +function errorMessage(node) { + return node + ' does not need shouldComponentUpdate when extending React.PureComponent.'; +} + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'Flag shouldComponentUpdate when extending PureComponent', + category: 'Possible Errors', + recommended: false + }, + schema: [] + }, + + create: Components.detect(function(context, components, utils) { + + /** + * Get properties name + * @param {Object} node - Property. + * @returns {String} Property name. + */ + function getPropertyName(node) { + // Special case for class properties + // (babel-eslint does not expose property name so we have to rely on tokens) + if (node.type === 'ClassProperty') { + var tokens = context.getFirstTokens(node, 2); + return tokens[1] && tokens[1].type === 'Identifier' ? tokens[1].value : tokens[0].value; + } + + return node.key.name; + } + + /** + * Get properties for a given AST node + * @param {ASTNode} node The AST node being checked. + * @returns {Array} Properties array. + */ + function getComponentProperties(node) { + switch (node.type) { + case 'ClassExpression': + case 'ClassDeclaration': + return node.body.body; + default: + return []; + } + } + + /** + * Checks for shouldComponentUpdate property + * @param {ASTNode} node The AST node being checked. + * @returns {Boolean} Whether or not the property exists. + */ + function hasShouldComponentUpdate(node) { + var properties = getComponentProperties(node); + return properties.some(function(property) { + var name = getPropertyName(property); + return name === 'shouldComponentUpdate'; + }); + } + + /** + * Checks for violation of rule + * @param {ASTNode} node The AST node being checked. + */ + function checkForViolation(node) { + if (utils.isPureComponent(node)) { + var hasScu = hasShouldComponentUpdate(node); + if (hasScu) { + var className = node.id.name; + context.report({ + node: node, + message: errorMessage(className) + }); + } + } + } + + return { + ClassDeclaration: checkForViolation, + ClassExpression: checkForViolation + }; + }) +}; diff --git a/tests/lib/rules/no-useless-scu.js b/tests/lib/rules/no-useless-scu.js new file mode 100644 index 0000000000..3d55dfa77f --- /dev/null +++ b/tests/lib/rules/no-useless-scu.js @@ -0,0 +1,128 @@ +/** + * @fileoverview Tests for no-useless-scu + */ + +'use strict'; + +// ----------------------------------------------------------------------------- +// Requirements +// ----------------------------------------------------------------------------- + +var rule = require('../../../lib/rules/no-useless-scu'); +var RuleTester = require('eslint').RuleTester; + +var parserOptions = { + ecmaVersion: 6, + ecmaFeatures: { + experimentalObjectRestSpread: true, + jsx: true + } +}; + +function errorMessage(node) { + return node + ' does not need shouldComponentUpdate when extending React.PureComponent.'; +} + +// ----------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------- + +var ruleTester = new RuleTester(); +ruleTester.run('no-useless-scu', rule, { + valid: [ + { + code: [ + 'class Foo extends React.Component {', + ' shouldComponentUpdate() {', + ' return true;', + ' }', + '}' + ].join('\n'), + parserOptions: parserOptions + }, + { + code: [ + 'class Foo extends React.Component {', + ' shouldComponentUpdate() {', + ' return true;', + ' }', + '}' + ].join('\n'), + parserOptions: parserOptions + }, + { + code: [ + 'function Foo() {', + ' return class Bar extends React.Component {', + ' shouldComponentUpdate() {', + ' return true;', + ' }', + ' };', + '}' + ].join('\n'), + parserOptions: parserOptions + } + ], + invalid: [ + { + code: [ + 'class Foo extends React.PureComponent {', + ' shouldComponentUpdate() {', + ' return true;', + ' }', + '}' + ].join('\n'), + errors: [{message: errorMessage('Foo')}], + parserOptions: parserOptions + }, + { + code: [ + 'class Foo extends PureComponent {', + ' shouldComponentUpdate() {', + ' return true;', + ' }', + '}' + ].join('\n'), + errors: [{message: errorMessage('Foo')}], + parserOptions: parserOptions + }, + { + code: [ + 'class Foo extends React.PureComponent {', + ' shouldComponentUpdate = () => {', + ' return true;', + ' }', + '}' + ].join('\n'), + errors: [{message: errorMessage('Foo')}], + parser: 'babel-eslint', + parserOptions: parserOptions + }, + { + code: [ + 'function Foo() {', + ' return class Bar extends React.PureComponent {', + ' shouldComponentUpdate() {', + ' return true;', + ' }', + ' };', + '}' + ].join('\n'), + errors: [{message: errorMessage('Bar')}], + parserOptions: parserOptions + }, + { + code: [ + 'function Foo() {', + ' return class Bar extends PureComponent {', + ' shouldComponentUpdate() {', + ' return true;', + ' }', + ' };', + '}' + ].join('\n'), + errors: [{message: errorMessage('Bar')}], + parserOptions: parserOptions + } + ] +});