Skip to content

Commit

Permalink
Add react/no-useless-scu rule
Browse files Browse the repository at this point in the history
This will check for `shouldComponentUpdate` defined in a class that
extends React.PureComponent.
  • Loading branch information
jomasti committed Feb 22, 2017
1 parent 8148833 commit 619110d
Show file tree
Hide file tree
Showing 5 changed files with 289 additions and 1 deletion.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 64 additions & 0 deletions docs/rules/no-useless-scu.md
Original file line number Diff line number Diff line change
@@ -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 <div>Radical!</div>
}
}

function Bar() {
return class Baz extends React.PureComponent {
shouldComponentUpdate() {
return true;
}

render() {
return <div>Groovy!</div>
}
}
}
```

The following patterns are not considered warnings:

```jsx
class Foo extends React.Component {
shouldComponentUpdate() {
return true;
}

render() {
return <div>Radical!</div>
}
}

function Bar() {
return class Baz extends React.Component {
shouldComponentUpdate() {
return true;
}

render() {
return <div>Groovy!</div>
}
}
}

class Qux extends React.PureComponent {
render() {
return <div>Tubular!</div>
}
}
```
3 changes: 2 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
94 changes: 94 additions & 0 deletions lib/rules/no-useless-scu.js
Original file line number Diff line number Diff line change
@@ -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
};
})
};
128 changes: 128 additions & 0 deletions tests/lib/rules/no-useless-scu.js
Original file line number Diff line number Diff line change
@@ -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
}
]
});

0 comments on commit 619110d

Please sign in to comment.