From 3d5301615218400408cbe760d808d3703af70d5b Mon Sep 17 00:00:00 2001 From: rickhanlonii Date: Sun, 9 Sep 2018 15:53:05 -0400 Subject: [PATCH] [New] Add no-pure-component-children rule --- README.md | 1 + docs/rules/no-pure-component-children.md | 54 +++ index.js | 1 + lib/rules/no-pure-component-children.js | 54 +++ tests/lib/rules/no-pure-component-children.js | 368 ++++++++++++++++++ 5 files changed, 478 insertions(+) create mode 100644 docs/rules/no-pure-component-children.md create mode 100644 lib/rules/no-pure-component-children.js create mode 100644 tests/lib/rules/no-pure-component-children.js diff --git a/README.md b/README.md index 5b787b1afc..a5e17ed345 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,7 @@ Enable the rules that you would like to use. * [react/no-find-dom-node](docs/rules/no-find-dom-node.md): Prevent usage of `findDOMNode` * [react/no-is-mounted](docs/rules/no-is-mounted.md): Prevent usage of `isMounted` * [react/no-multi-comp](docs/rules/no-multi-comp.md): Prevent multiple component definition per file +* [react/no-pure-component-children](docs/rules/no-pure-component-children.md): Prevent using `React.PureComponent` with children * [react/no-redundant-should-component-update](docs/rules/no-redundant-should-component-update.md): Prevent usage of `shouldComponentUpdate` when extending React.PureComponent * [react/no-render-return-value](docs/rules/no-render-return-value.md): Prevent usage of the return value of `React.render` * [react/no-set-state](docs/rules/no-set-state.md): Prevent usage of `setState` diff --git a/docs/rules/no-pure-component-children.md b/docs/rules/no-pure-component-children.md new file mode 100644 index 0000000000..48202c5872 --- /dev/null +++ b/docs/rules/no-pure-component-children.md @@ -0,0 +1,54 @@ +# Prevent using React.PureComponent with children (react/no-pure-component-children) + +Warns if you have defined children PropTypes when defining a component that extends React.PureComponent. +Children props are almost never equal, and when they're not, React.PureComponent performs worse than React.Component. + +## Rule Details + +The following patterns are considered warnings: + +```jsx +class Foo extends React.PureComponent { + render() { + return
Radical!
+ } +} + +Foo.propTypes = { + children: PropTypes.node, +}; + +class Bar extends React.PureComponent { + static propTypes = { + children: PropTypes.node, + }; + + render() { + return
Groovy!
+ } +} +``` + +The following patterns are **not** considered warnings: + +```jsx +class Foo extends React.Component { + render() { + return
Radical!
+ } +} + +Foo.propTypes = { + children: PropTypes.node, +}; + +class Bar extends React.Component { + static propTypes = { + children: PropTypes.node, + }; + + render() { + return
Groovy!
+ } +} +``` diff --git a/index.js b/index.js index 45a96d7da3..6af2b9e516 100644 --- a/index.js +++ b/index.js @@ -56,6 +56,7 @@ const allRules = { 'no-find-dom-node': require('./lib/rules/no-find-dom-node'), 'no-is-mounted': require('./lib/rules/no-is-mounted'), 'no-multi-comp': require('./lib/rules/no-multi-comp'), + 'no-pure-component-children': require('./lib/rules/no-pure-component-children'), 'no-set-state': require('./lib/rules/no-set-state'), 'no-string-refs': require('./lib/rules/no-string-refs'), 'no-redundant-should-component-update': require('./lib/rules/no-redundant-should-component-update'), diff --git a/lib/rules/no-pure-component-children.js b/lib/rules/no-pure-component-children.js new file mode 100644 index 0000000000..b0fd018f25 --- /dev/null +++ b/lib/rules/no-pure-component-children.js @@ -0,0 +1,54 @@ +/** + * @fileoverview Prevent using React.PureComponent with children + */ +'use strict'; + +const Components = require('../util/Components'); +const docsUrl = require('../util/docsUrl'); +const has = require('has'); + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ +module.exports = { + meta: { + docs: { + description: 'Prevent using React.PureComponent with children', + category: 'Best Practices', + recommended: false, + url: docsUrl('no-pure-component-children') + }, + schema: [] + }, + create: Components.detect((context, components, utils) => ({ + ClassDeclaration: function(node) { + if (utils.isPureComponent(node)) { + components.set(node, { + isPureComponent: true + }); + } + }, + 'Program:exit': function() { + const list = components.list(); + + for (const key in list) { + if (!has(list, key)) { + continue; + } + + const component = list[key]; + if (!component.declaredPropTypes || !component.isPureComponent) { + continue; + } + + if (component.declaredPropTypes.children) { + context.report( + component.declaredPropTypes.children.node, + 'Do not use children with PureComponent', + {name: 'children'} + ); + } + } + } + })) +}; diff --git a/tests/lib/rules/no-pure-component-children.js b/tests/lib/rules/no-pure-component-children.js new file mode 100644 index 0000000000..d3ea682220 --- /dev/null +++ b/tests/lib/rules/no-pure-component-children.js @@ -0,0 +1,368 @@ +/** + * @fileoverview Tests for no-pure-component-children + */ +'use strict'; + +// ----------------------------------------------------------------------------- +// Requirements +// ----------------------------------------------------------------------------- + +const rule = require('../../../lib/rules/no-pure-component-children'); +const RuleTester = require('eslint').RuleTester; +require('babel-eslint'); + +const ERROR_MESSAGE = 'Do not use children with PureComponent'; + +const parserOptions = { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + jsx: true + } +}; + +// ----------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------- +const ruleTester = new RuleTester({parserOptions}); +ruleTester.run('no-pure-component-children', rule, { + valid: [{ + code: ` + var First = createReactClass({ + render: function() { + return
; + } + }); + ` + }, { + code: ` + var First = createReactClass({ + propTypes: externalPropTypes, + render: function() { + return
; + } + }); + ` + }, { + code: ` + var First = createReactClass({ + propTypes: { + foo: PropTypes.string + }, + render: function() { + return
; + } + }); + ` + }, { + code: ` + var First = createReactClass({ + propTypes: { + children: PropTypes.string + }, + render: function() { + return
; + } + }); + ` + }, { + code: ` + var First = createReactClass({ + propTypes: { + foo: PropTypes.string + }, + render: function() { + return
; + } + }); + + var Second = createReactClass({ + propTypes: { + bar: PropTypes.string + }, + render: function() { + return
; + } + }); + ` + }, { + code: ` + class Component extends React.Component { + render() { + return
; + } + } + ` + }, { + code: ` + class Component extends React.Component { + render() { + return
; + } + } + Component.propTypes = { + foo: PropTypes.node, + }; + ` + }, { + code: ` + class Component extends React.Component { + render() { + return
; + } + } + + Component.propTypes = { + children: PropTypes.node, + }; + ` + }, { + code: ` + class Component extends React.PureComponent { + render() { + return
; + } + } + Component.propTypes = { + foo: PropTypes.node, + }; + ` + }, { + code: ` + class FirstComponent extends React.Component { + render() { + return
; + } + } + + FirstComponent.propTypes = { + children: PropTypes.node, + }; + + class SecondComponent extends React.PureComponent { + render() { + return
; + } + } + SecondComponent.propTypes = { + foo: PropTypes.node, + }; + ` + }, { + parser: 'babel-eslint', + code: ` + class Component extends React.Component { + static propTypes = { + foo: PropTypes.node, + }; + render() { + return
; + } + } + ` + }, { + parser: 'babel-eslint', + code: ` + class Component extends React.Component { + static propTypes = { + children: PropTypes.node, + }; + render() { + return
; + } + } + ` + }, { + parser: 'babel-eslint', + code: ` + class Component extends React.PureComponent { + static propTypes = { + foo: PropTypes.node, + }; + render() { + return
; + } + } + ` + }, { + parser: 'babel-eslint', + code: ` + class FirstComponent extends React.Component { + static propTypes = { + children: PropTypes.node, + }; + render() { + return
; + } + } + + class SecondComponent extends React.PureComponent { + static propTypes = { + foo: PropTypes.node, + }; + render() { + return
; + } + } + ` + }], + invalid: [{ + code: ` + class Component extends React.PureComponent { + render() { + return
; + } + } + Component.propTypes = { + children: PropTypes.node, + }; + `, + errors: [{ + message: ERROR_MESSAGE, + line: 8, + column: 19, + type: 'MemberExpression' + }], + output: ` + class Component extends React.PureComponent { + render() { + return
; + } + } + Component.propTypes = { + children: PropTypes.node, + }; + ` + }, { + code: ` + class FirstComponent extends React.PureComponent { + render() { + return
; + } + } + FirstComponent.propTypes = { + children: PropTypes.node, + }; + + class SecondComponent extends React.PureComponent { + render() { + return
; + } + } + SecondComponent.propTypes = { + children: PropTypes.node, + }; + `, + errors: [{ + message: ERROR_MESSAGE, + line: 8, + column: 19, + type: 'MemberExpression' + }, { + message: ERROR_MESSAGE, + line: 17, + column: 19, + type: 'MemberExpression' + }], + output: ` + class FirstComponent extends React.PureComponent { + render() { + return
; + } + } + FirstComponent.propTypes = { + children: PropTypes.node, + }; + + class SecondComponent extends React.PureComponent { + render() { + return
; + } + } + SecondComponent.propTypes = { + children: PropTypes.node, + }; + ` + }, + { + parser: 'babel-eslint', + code: ` + class Component extends React.PureComponent { + static propTypes = { + children: PropTypes.node, + }; + render() { + return
; + } + } + `, + errors: [{ + message: ERROR_MESSAGE, + line: 4, + column: 21, + type: 'MemberExpression' + }], + output: ` + class Component extends React.PureComponent { + static propTypes = { + children: PropTypes.node, + }; + render() { + return
; + } + } + ` + }, + { + parser: 'babel-eslint', + code: ` + class FirstComponent extends React.PureComponent { + static propTypes = { + children: PropTypes.node, + }; + render() { + return
; + } + } + + class SecondComponent extends React.PureComponent { + static propTypes = { + children: PropTypes.node, + }; + render() { + return
; + } + } + `, + errors: [{ + message: ERROR_MESSAGE, + line: 4, + column: 21, + type: 'MemberExpression' + }, { + message: ERROR_MESSAGE, + line: 13, + column: 21, + type: 'MemberExpression' + }], + output: ` + class FirstComponent extends React.PureComponent { + static propTypes = { + children: PropTypes.node, + }; + render() { + return
; + } + } + + class SecondComponent extends React.PureComponent { + static propTypes = { + children: PropTypes.node, + }; + render() { + return
; + } + } + ` + }] +});