From 6fdcc932bc1bb557c39e49e352c3676b80220755 Mon Sep 17 00:00:00 2001 From: Benjamin Stepp Date: Tue, 26 Jul 2016 23:56:50 -0500 Subject: [PATCH] Add new no-children-prop rule (Fixes #720) Prevents children being passed as props. Children should be actual children. --- * Add Component to tests for no-children-prop * Add note in docs about recommended behavior * Change syntax in docs for no-children-prop to jsx --- * Clarify that multiple arguments may be passed as children to React.createElement * Reorganize tests to show JSX with createElement counterpart immediately following. * Added test cases with a non-children property * Added test cases with multiple children --- README.md | 1 + docs/rules/no-children-prop.md | 36 +++++ index.js | 3 +- lib/rules/no-children-prop.js | 65 ++++++++ tests/lib/rules/no-children-prop.js | 237 ++++++++++++++++++++++++++++ 5 files changed, 341 insertions(+), 1 deletion(-) create mode 100644 docs/rules/no-children-prop.md create mode 100644 lib/rules/no-children-prop.js create mode 100644 tests/lib/rules/no-children-prop.js diff --git a/README.md b/README.md index fb08b1cb6a..3bb37e6237 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](# * [react/display-name](docs/rules/display-name.md): Prevent missing `displayName` in a React component definition * [react/forbid-prop-types](docs/rules/forbid-prop-types.md): Forbid certain propTypes +* [react/no-children-prop](docs/rules/no-children-prop.md): Prevent passing children as props * [react/no-danger](docs/rules/no-danger.md): Prevent usage of dangerous JSX properties * [react/no-deprecated](docs/rules/no-deprecated.md): Prevent usage of deprecated methods * [react/no-did-mount-set-state](docs/rules/no-did-mount-set-state.md): Prevent usage of `setState` in `componentDidMount` diff --git a/docs/rules/no-children-prop.md b/docs/rules/no-children-prop.md new file mode 100644 index 0000000000..e49c7d8715 --- /dev/null +++ b/docs/rules/no-children-prop.md @@ -0,0 +1,36 @@ +# Prevent passing of children as props (no-children-prop) + +Children should always be actual children, not passed in as a prop. + +When using JSX, the children should be nested between the opening and closing +tags. When not using JSX, the children should be passed as additional +arguments to `React.createElement`. + +## Rule Details + +The following patterns are considered warnings: + +```jsx +
+ +} /> + + +React.createElement("div", { children: 'Children' }) +``` + +The following patterns are not considered warnings: + +```jsx +
Children
+ +Children + + + Child 1 + Child 2 + + +React.createElement("div", {}, 'Children') +React.createElement("div", 'Child 1', 'Child 2') +``` diff --git a/index.js b/index.js index a28494f79e..effbfa0a21 100644 --- a/index.js +++ b/index.js @@ -50,7 +50,8 @@ var rules = { 'jsx-no-target-blank': require('./lib/rules/jsx-no-target-blank'), 'jsx-filename-extension': require('./lib/rules/jsx-filename-extension'), 'require-optimization': require('./lib/rules/require-optimization'), - 'no-find-dom-node': require('./lib/rules/no-find-dom-node') + 'no-find-dom-node': require('./lib/rules/no-find-dom-node'), + 'no-children-prop': require('./lib/rules/no-children-prop') }; var ruleNames = Object.keys(rules); diff --git a/lib/rules/no-children-prop.js b/lib/rules/no-children-prop.js new file mode 100644 index 0000000000..efead3b2f7 --- /dev/null +++ b/lib/rules/no-children-prop.js @@ -0,0 +1,65 @@ +/** + * @fileoverview Prevent passing of children as props + * @author Benjamin Stepp + */ +'use strict'; + +// ------------------------------------------------------------------------------ +// Helpers +// ------------------------------------------------------------------------------ + +/** + * Checks if the node is a createElement call with a props literal. + * @param {ASTNode} node - The AST node being checked. + * @returns {Boolean} - True if node is a createElement call with a props + * object literal, False if not. +*/ +function isCreateElementWithProps(node) { + return node.callee + && node.callee.type === 'MemberExpression' + && node.callee.property.name === 'createElement' + && node.arguments.length > 1 + && node.arguments[1].type === 'ObjectExpression'; +} + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: {}, + schema: [] + }, + create: function(context) { + return { + JSXAttribute: function(node) { + if (node.name.name !== 'children') { + return; + } + + context.report({ + node: node, + message: 'Do not pass children as props. Instead, nest children between the opening and closing tags.' + }); + }, + CallExpression: function(node) { + if (!isCreateElementWithProps(node)) { + return; + } + + var props = node.arguments[1].properties; + var childrenProp = props.find(function(prop) { + return prop.key.name === 'children'; + }); + + if (childrenProp) { + context.report({ + node: node, + message: 'Do not pass children as props. Instead, pass them as additional arguments to React.createElement.' + }); + } + } + }; + } +}; diff --git a/tests/lib/rules/no-children-prop.js b/tests/lib/rules/no-children-prop.js new file mode 100644 index 0000000000..d0073dc0f2 --- /dev/null +++ b/tests/lib/rules/no-children-prop.js @@ -0,0 +1,237 @@ +/** + * @fileoverview Tests for no-children-prop + * @author Benjamin Stepp + */ + +'use strict'; + +// ----------------------------------------------------------------------------- +// Requirements +// ----------------------------------------------------------------------------- + +var rule = require('../../../lib/rules/no-children-prop'); +var RuleTester = require('eslint').RuleTester; + +var parserOptions = { + ecmaVersion: 6, + ecmaFeatures: { + jsx: true + } +}; + +var JSX_ERROR = 'Do not pass children as props. Instead, nest children between \ +the opening and closing tags.'; +var CREATE_ELEMENT_ERROR = 'Do not pass children as props. Instead, pass them \ +as additional arguments to React.createElement.'; + +// ----------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------- + +var ruleTester = new RuleTester(); +ruleTester.run('no-children-prop', rule, { + valid: [ + { + code: '
;', + parserOptions: parserOptions + }, + { + code: '
;', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {});', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", undefined);', + parserOptions: parserOptions + }, + { + code: '
;', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {className: "class-name"});', + parserOptions: parserOptions + }, + { + code: '
Children
;', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", "Children");', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {}, "Children");', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", undefined, "Children");', + parserOptions: parserOptions + }, + { + code: '
Children
;', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {className: "class-name"}, "Children");', + parserOptions: parserOptions + }, + { + code: '
;', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", React.createElement("div"));', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {}, React.createElement("div"));', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", undefined, React.createElement("div"));', + parserOptions: parserOptions + }, + { + code: '
;', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", React.createElement("div"), React.createElement("div"));', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {}, React.createElement("div"), React.createElement("div"));', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", undefined, React.createElement("div"), React.createElement("div"));', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", [React.createElement("div"), React.createElement("div")]);', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {}, [React.createElement("div"), React.createElement("div")]);', + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", undefined, [React.createElement("div"), React.createElement("div")]);', + parserOptions: parserOptions + }, + { + code: '', + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent);', + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, {});', + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, undefined);', + parserOptions: parserOptions + }, + { + code: 'Children;', + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, "Children");', + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, {}, "Children");', + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, undefined, "Children");', + parserOptions: parserOptions + }, + { + code: ';', + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, {className: "class-name"});', + parserOptions: parserOptions + }, + { + code: 'Children;', + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, {className: "class-name"}, "Children");', + parserOptions: parserOptions + } + ], + invalid: [ + { + code: '
;', + errors: [{message: JSX_ERROR}], + parserOptions: parserOptions + }, + { + code: '
} />;', + errors: [{message: JSX_ERROR}], + parserOptions: parserOptions + }, + { + code: '
,
]} />;', + errors: [{message: JSX_ERROR}], + parserOptions: parserOptions + }, + { + code: '
Children
;', + errors: [{message: JSX_ERROR}], + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {children: "Children"});', + errors: [{message: CREATE_ELEMENT_ERROR}], + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {children: "Children"}, "Children");', + errors: [{message: CREATE_ELEMENT_ERROR}], + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {children: React.createElement("div")});', + errors: [{message: CREATE_ELEMENT_ERROR}], + parserOptions: parserOptions + }, + { + code: 'React.createElement("div", {children: [React.createElement("div"), React.createElement("div")]});', + errors: [{message: CREATE_ELEMENT_ERROR}], + parserOptions: parserOptions + }, + { + code: '', + errors: [{message: JSX_ERROR}], + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, {children: "Children"});', + errors: [{message: CREATE_ELEMENT_ERROR}], + parserOptions: parserOptions + }, + { + code: ';', + errors: [{message: JSX_ERROR}], + parserOptions: parserOptions + }, + { + code: 'React.createElement(MyComponent, {children: "Children", className: "class-name"});', + errors: [{message: CREATE_ELEMENT_ERROR}], + parserOptions: parserOptions + } + ] +});