diff --git a/README.md b/README.md index 883b4106da..851fc852eb 100644 --- a/README.md +++ b/README.md @@ -121,6 +121,7 @@ Enable the rules that you would like to use. * [react/no-unused-state](docs/rules/no-unused-state.md): Prevent definitions of unused state properties * [react/no-will-update-set-state](docs/rules/no-will-update-set-state.md): Prevent usage of `setState` in `componentWillUpdate` * [react/prefer-es6-class](docs/rules/prefer-es6-class.md): Enforce ES5 or ES6 class for React Components +* [react/prefer-separate-components](docs/rules/prefer-separate-components.md): Report usages of class methods besides render returning JSX * [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 * [react/react-in-jsx-scope](docs/rules/react-in-jsx-scope.md): Prevent missing `React` when using JSX diff --git a/docs/rules/prefer-separate-components.md b/docs/rules/prefer-separate-components.md new file mode 100644 index 0000000000..5fd1113076 --- /dev/null +++ b/docs/rules/prefer-separate-components.md @@ -0,0 +1,95 @@ +# Report usages of class methods besides render returning JSX (react/prefer-separate-components) + +If component functions other than `render` are returning JSX, they should be moved into separate components. + +## Rule Details + +The following patterns are considered warnings: + +```jsx +class Foo extends React.Component { + renderBar() { + return bar; + } + render() { + return ( +
+ foo + {this.renderBar()} +
+ ); + } +} + +class Foo extends React.Component { + renderBar() { + return React.createElement('span', null, 'bar'); + } + render() { + return React.createElement( + 'div', + null, + 'foo', + this.renderBar() + ); + } +} + +class Foo extends React.Component { + renderBars() { + const bars = ['bar', 'bar', 'bar']; + return bars.map((bar) => {bar}); + } + render() { + return ( +
+ foo + {this.renderBars()} +
+ ); + } +} + +class Foo extends React.Component { + renderBars() { + const bars = ['bar', 'bar', 'bar']; + return bars.map((bar) => React.createElement('span', null, bar)); + } + render() { + return React.createElement( + 'div', + null, + 'foo', + this.renderBars() + ); + } +} +``` + +The following patterns are **not** considered warnings: + +```jsx +class Foo extends React.Component { + render() { + return
foo
; + } +} + +class Foo extends React.Component { + render() { + return React.createElement('div', null, 'foo'); + } +} + +createReactClass({ + render() { + return
foo
; + } +}); + +createReactClass({ + render() { + return React.createElement('div', null, 'foo'); + } +}); +``` diff --git a/index.js b/index.js index 1b3a1238da..c01b553cc0 100644 --- a/index.js +++ b/index.js @@ -65,6 +65,7 @@ const allRules = { 'no-unused-state': require('./lib/rules/no-unused-state'), 'no-will-update-set-state': require('./lib/rules/no-will-update-set-state'), 'prefer-es6-class': require('./lib/rules/prefer-es6-class'), + 'prefer-separate-components': require('./lib/rules/prefer-separate-components'), 'prefer-stateless-function': require('./lib/rules/prefer-stateless-function'), 'prop-types': require('./lib/rules/prop-types'), 'react-in-jsx-scope': require('./lib/rules/react-in-jsx-scope'), diff --git a/lib/rules/prefer-separate-components.js b/lib/rules/prefer-separate-components.js new file mode 100644 index 0000000000..f5766ebd8d --- /dev/null +++ b/lib/rules/prefer-separate-components.js @@ -0,0 +1,53 @@ +/** + * @fileoverview Report usages of class methods besides render returning JSX + */ +'use strict'; + +const Components = require('../util/Components'); + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'Report usages of class methods besides render returning JSX', + category: 'Best Practices', + recommended: false + }, + schema: [] + }, + + create: Components.detect((context, components, utils) => { + function report(node) { + context.report({ + node, + message: '{{ function }} should be moved into a separate component.', + data: { + function: node.key.name + } + }); + } + + return { + MethodDefinition: function(node) { + if (!utils.getParentES6Component()) { + return; + } + if (node.key.name !== 'render' && utils.isReturningJSX(node)) { + report(node); + } + }, + + Property: function(node) { + if (!utils.getParentES5Component) { + return; + } + if (node.key.name !== 'render' && utils.isReturningJSX(node)) { + report(node); + } + } + }; + }) +}; diff --git a/lib/util/Components.js b/lib/util/Components.js index 90ec38550f..18e985d36f 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -303,6 +303,16 @@ function componentRule(rule, context) { return calledOnReact; }, + isReturningArrayMap(node) { + return ( + node.type === 'ReturnStatement' && + node.argument && + node.argument.type === 'CallExpression' && + node.argument.callee.property && + node.argument.callee.property.name === 'map' + ); + }, + getReturnPropertyAndNode(ASTnode) { let property; let node = ASTnode; @@ -321,6 +331,9 @@ function componentRule(rule, context) { node = utils.findReturnStatement(node); property = 'argument'; } + if (this.isReturningArrayMap(node)) { + return this.getReturnPropertyAndNode(node.argument.arguments[0]); + } return { node: node, property: property diff --git a/tests/lib/rules/prefer-separate-components.js b/tests/lib/rules/prefer-separate-components.js new file mode 100644 index 0000000000..15b1771d3a --- /dev/null +++ b/tests/lib/rules/prefer-separate-components.js @@ -0,0 +1,198 @@ +/** + * @fileoverview Report usages of class methods besides render returning JSX + */ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/prefer-separate-components'); +const RuleTester = require('eslint').RuleTester; + +const parserOptions = { + ecmaVersion: 8, + sourceType: 'module', + ecmaFeatures: { + experimentalObjectRestSpread: true, + jsx: true + } +}; + +const ruleTester = new RuleTester({parserOptions}); +ruleTester.run('prefer-separate-components', rule, { + valid: [{ + code: ` + class Foo extends React.Component { + render() { + return
foo
; + } + } + ` + }, { + code: ` + class Foo extends React.Component { + render() { + return React.createElement('div', null, 'foo'); + } + } + ` + }, { + code: ` + createReactClass({ + render() { + return
foo
; + } + }); + ` + }, { + code: ` + createReactClass({ + render() { + return React.createElement('div', null, 'foo'); + } + }); + ` + }], + invalid: [{ + code: ` + class Foo extends React.Component { + renderBar() { + return bar; + } + render() { + return ( +
+ foo + {this.renderBar()} +
+ ); + } + } + `, + errors: 1 + }, { + code: ` + class Foo extends React.Component { + renderBar() { + return React.createElement('span', null, 'bar'); + } + render() { + return React.createElement( + 'div', + null, + 'foo', + this.renderBar() + ); + } + } + `, + errors: 1 + }, { + code: ` + class Foo extends React.Component { + renderBars() { + const bars = ['bar', 'bar', 'bar']; + return bars.map((bar) => {bar}); + } + render() { + return ( +
+ foo + {this.renderBars()} +
+ ); + } + } + `, + errors: 1 + }, { + code: ` + class Foo extends React.Component { + renderBars() { + const bars = ['bar', 'bar', 'bar']; + return bars.map((bar) => React.createElement('span', null, bar)); + } + render() { + return React.createElement( + 'div', + null, + 'foo', + this.renderBars() + ); + } + } + `, + errors: 1 + }, { + code: ` + createReactClass({ + renderBars() { + const bars = ['bar', 'bar', 'bar']; + return bars.map((bar) => {bar}); + }, + render() { + return ( +
+ foo + {this.renderBars()} +
+ ); + } + }); + `, + errors: 1 + }, { + code: ` + createReactClass({ + renderBars() { + const bars = ['bar', 'bar', 'bar']; + return bars.map((bar) => React.createElement('span', null, bar)); + }, + render() { + return React.createElement( + 'div', + null, + 'foo', + this.renderBars() + ); + } + }); + `, + errors: 1 + }, { + code: ` + createReactClass({ + renderBar() { + return bar; + }, + render() { + return ( +
+ foo + {this.renderBar()} +
+ ); + } + }); + `, + errors: 1 + }, { + code: ` + createReactClass({ + renderBar() { + return React.createElement('span', null, 'bar'); + }, + render() { + return React.createElement( + 'div', + null, + 'foo', + this.renderBar() + ); + } + }); + `, + errors: 1 + }] +}); diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index 08153a711a..77eb4aff69 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -1273,15 +1273,6 @@ ruleTester.run('prop-types', rule, { ' name: PropTypes.string', '}' ].join('\n') - }, { - code: [ - 'function Hello({names}) {', - ' return names.map((name) => {', - ' return
{name}
;', - ' });', - '}' - ].join('\n'), - parser: 'babel-eslint' }, { code: [ 'type Target = { target: EventTarget }', @@ -3533,6 +3524,18 @@ ruleTester.run('prop-types', rule, { message: '\'fooBar\' is missing in props validation' }], parser: 'babel-eslint' + }, { + code: [ + 'function Hello({names}) {', + ' return names.map((name) => {', + ' return
{name}
;', + ' });', + '}' + ].join('\n'), + errors: [{ + message: '\'names\' is missing in props validation' + }], + parser: 'babel-eslint' } ] });