-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fixes #240. Adds new require-optimization rule.
- Loading branch information
Evgueni Naverniouk
committed
May 15, 2016
1 parent
750f979
commit 547cb5f
Showing
5 changed files
with
325 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
# Enforce React components to have a shouldComponentUpdate method (require-optimization) | ||
|
||
This rule prevents you from creating React components without declaring a `shouldComponentUpdate` method. | ||
|
||
## Rule Details | ||
|
||
The following patterns are considered warnings: | ||
|
||
```js | ||
class YourComponent extends React.Component { | ||
|
||
} | ||
``` | ||
|
||
```js | ||
React.createClass({ | ||
}); | ||
``` | ||
|
||
The following patterns are not considered warnings: | ||
|
||
```js | ||
class YourComponent extends React.Component { | ||
shouldComponentUpdate () { | ||
return false; | ||
} | ||
} | ||
``` | ||
|
||
```js | ||
React.createClass({ | ||
shouldComponentUpdate: function () { | ||
return false; | ||
} | ||
}); | ||
``` | ||
|
||
```js | ||
React.createClass({ | ||
mixins: [PureRenderMixin] | ||
}); | ||
``` | ||
|
||
```js | ||
@reactMixin.decorate(PureRenderMixin) | ||
React.createClass({ | ||
|
||
}); | ||
``` | ||
|
||
## Rule Options | ||
|
||
```js | ||
... | ||
"require-optimization": [<enabled>] | ||
... | ||
``` | ||
|
||
### Example | ||
|
||
```js | ||
... | ||
"require-optimization": 2 | ||
... | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
/** | ||
* @fileoverview Enforce React components to have a shouldComponentUpdate method | ||
* @author Evgueni Naverniouk | ||
*/ | ||
'use strict'; | ||
|
||
var Components = require('../util/Components'); | ||
|
||
module.exports = Components.detect(function (context, components) { | ||
var MISSING_MESSAGE = 'Component is not optimized. Please add a shouldComponentUpdate method.'; | ||
|
||
/** | ||
* Checks to see if our component is decorated by PureRenderMixin via reactMixin | ||
* @param {ASTNode} node The AST node being checked. | ||
* @returns {Boolean} True if node is decorated with a PureRenderMixin, false if not. | ||
*/ | ||
var hasPureRenderDecorator = function (node) { | ||
if (node.decorators && node.decorators.length) { | ||
for (var i = 0, l = node.decorators.length; i < l; i++) { | ||
if ( | ||
node.decorators[i].expression && | ||
node.decorators[i].expression.callee && | ||
node.decorators[i].expression.callee.object && | ||
node.decorators[i].expression.callee.object.name === 'reactMixin' && | ||
node.decorators[i].expression.callee.property && | ||
node.decorators[i].expression.callee.property.name === 'decorate' && | ||
node.decorators[i].expression.arguments && | ||
node.decorators[i].expression.arguments.length && | ||
node.decorators[i].expression.arguments[0].name === 'PureRenderMixin' | ||
) { | ||
return true; | ||
} | ||
} | ||
} | ||
|
||
return false; | ||
}; | ||
|
||
/** | ||
* Checks if we are declaring a shouldComponentUpdate method | ||
* @param {ASTNode} node The AST node being checked. | ||
* @returns {Boolean} True if we are declaring a shouldComponentUpdate method, false if not. | ||
*/ | ||
var isSCUDeclarеd = function (node) { | ||
return Boolean( | ||
node && | ||
node.name === 'shouldComponentUpdate' | ||
); | ||
}; | ||
|
||
/** | ||
* Checks if we are declaring a PureRenderMixin mixin | ||
* @param {ASTNode} node The AST node being checked. | ||
* @returns {Boolean} True if we are declaring a PureRenderMixin method, false if not. | ||
*/ | ||
var isPureRenderDeclared = function (node) { | ||
var hasPR = false; | ||
if (node.value && node.value.elements) { | ||
for (var i = 0, l = node.value.elements.length; i < l; i++) { | ||
if (node.value.elements[i].name === 'PureRenderMixin') { | ||
hasPR = true; | ||
break; | ||
} | ||
} | ||
} | ||
|
||
return Boolean( | ||
node && | ||
node.key.name === 'mixins' && | ||
hasPR | ||
); | ||
}; | ||
|
||
/** | ||
* Mark shouldComponentUpdate as declared | ||
* @param {ASTNode} node The AST node being checked. | ||
*/ | ||
var markSCUAsDeclared = function (node) { | ||
components.set(node, { | ||
hasSCU: true | ||
}); | ||
}; | ||
|
||
/** | ||
* Reports missing optimization for a given component | ||
* @param {Object} component The component to process | ||
*/ | ||
var reportMissingOptimization = function (component) { | ||
context.report({ | ||
node: component.node, | ||
message: MISSING_MESSAGE, | ||
data: { | ||
component: component.name | ||
} | ||
}); | ||
}; | ||
|
||
return { | ||
ClassDeclaration: function (node) { | ||
if (!hasPureRenderDecorator(node)) { | ||
return; | ||
} | ||
markSCUAsDeclared(node); | ||
}, | ||
|
||
MethodDefinition: function (node) { | ||
if (!isSCUDeclarеd(node.key)) { | ||
return; | ||
} | ||
markSCUAsDeclared(node); | ||
}, | ||
|
||
ObjectExpression: function (node) { | ||
// Search for the shouldComponentUpdate declaration | ||
for (var i = 0, l = node.properties.length; i < l; i++) { | ||
if ( | ||
!node.properties[i].key || ( | ||
!isSCUDeclarеd(node.properties[i].key) && | ||
!isPureRenderDeclared(node.properties[i]) | ||
) | ||
) { | ||
continue; | ||
} | ||
markSCUAsDeclared(node); | ||
} | ||
}, | ||
|
||
'Program:exit': function () { | ||
var list = components.list(); | ||
|
||
// Report missing shouldComponentUpdate for all components | ||
for (var component in list) { | ||
if (!list.hasOwnProperty(component) || list[component].hasSCU) { | ||
continue; | ||
} | ||
reportMissingOptimization(list[component]); | ||
} | ||
} | ||
}; | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
/** | ||
* @fileoverview Enforce React components to have a shouldComponentUpdate method | ||
* @author Evgueni Naverniouk | ||
*/ | ||
'use strict'; | ||
|
||
var rule = require('../../../lib/rules/require-optimization'); | ||
var RuleTester = require('eslint').RuleTester; | ||
|
||
var parserOptions = { | ||
ecmaVersion: 6, | ||
sourceType: 'module' | ||
}; | ||
|
||
var MESSAGE = 'Component is not optimized. Please add a shouldComponentUpdate method.'; | ||
|
||
var ruleTester = new RuleTester(); | ||
ruleTester.run('react-require-optimization', rule, { | ||
valid: [{ | ||
code: [ | ||
'class A {}' | ||
].join('\n'), | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'import React from "react";' + | ||
'class YourComponent extends React.Component {' + | ||
'shouldComponentUpdate () {}' + | ||
'}' | ||
].join('\n'), | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'import React, {Component} from "react";' + | ||
'class YourComponent extends Component {' + | ||
'shouldComponentUpdate () {}' + | ||
'}' | ||
].join('\n'), | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'import React from "react";' + | ||
'React.createClass({' + | ||
'shouldComponentUpdate: function () {}' + | ||
'})' | ||
].join('\n'), | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'import React from "react";' + | ||
'React.createClass({' + | ||
'mixins: [PureRenderMixin]' + | ||
'})' | ||
].join('\n'), | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'@reactMixin.decorate(PureRenderMixin)', | ||
'class DecoratedComponent extends Component {' + | ||
'}' | ||
].join('\n'), | ||
parser: 'babel-eslint', | ||
parserOptions: parserOptions | ||
}], | ||
|
||
invalid: [{ | ||
code: [ | ||
'import React from "react";' + | ||
'class YourComponent extends React.Component {}' | ||
].join('\n'), | ||
errors: [{ | ||
message: MESSAGE | ||
}], | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'import React, {Component} from "react";' + | ||
'class YourComponent extends Component {}' | ||
].join('\n'), | ||
errors: [{ | ||
message: MESSAGE | ||
}], | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'import React from "react";' + | ||
'React.createClass({' + | ||
'})' | ||
].join('\n'), | ||
errors: [{ | ||
message: MESSAGE | ||
}], | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'import React from "react";' + | ||
'React.createClass({' + | ||
'mixins: [RandomMixin]' + | ||
'})' | ||
].join('\n'), | ||
errors: [{ | ||
message: MESSAGE | ||
}], | ||
parserOptions: parserOptions | ||
}, { | ||
code: [ | ||
'@reactMixin.decorate(SomeOtherMixin)', | ||
'class DecoratedComponent extends Component {' + | ||
'}' | ||
].join('\n'), | ||
errors: [{ | ||
message: MESSAGE | ||
}], | ||
parser: 'babel-eslint', | ||
parserOptions: parserOptions | ||
}] | ||
}); |