Skip to content

Commit

Permalink
Add new no-children-prop rule (Fixes #720) (#722)
Browse files Browse the repository at this point in the history
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

---

* Add no-children-prop to recommended config

---

* Remove recommended config for no-children-prop
  • Loading branch information
benstepp authored and lencioni committed Sep 16, 2016
1 parent 21a3339 commit 0348173
Show file tree
Hide file tree
Showing 5 changed files with 346 additions and 1 deletion.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,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-component-props](docs/rules/forbid-component-props.md): Forbid certain props on Components
* [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-danger-with-children](docs/rules/no-danger-with-children.md): Prevent problem with children and props.dangerouslySetInnerHTML
* [react/no-deprecated](docs/rules/no-deprecated.md): Prevent usage of deprecated methods
Expand Down Expand Up @@ -166,6 +167,7 @@ The rules enabled in this configuration are:
* [react/jsx-no-undef](docs/rules/jsx-no-undef.md)
* [react/jsx-uses-react](docs/rules/jsx-uses-react.md)
* [react/jsx-uses-vars](docs/rules/jsx-uses-vars.md)
* [react/no-danger](docs/rules/no-danger.md)
* [react/no-deprecated](docs/rules/no-deprecated.md)
* [react/no-direct-mutation-state](docs/rules/no-direct-mutation-state.md)
* [react/no-find-dom-node](docs/rules/no-find-dom-node.md)
Expand Down
36 changes: 36 additions & 0 deletions docs/rules/no-children-prop.md
Original file line number Diff line number Diff line change
@@ -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
<div children='Children' />

<MyComponent children={<AnotherComponent />} />
<MyComponent children={['Child 1', 'Child 2']} />

React.createElement("div", { children: 'Children' })
```

The following patterns are not considered warnings:

```jsx
<div>Children</div>

<MyComponent>Children</MyComponent>

<MyComponent>
<span>Child 1</span>
<span>Child 2</span>
</MyComponent>

React.createElement("div", {}, 'Children')
React.createElement("div", 'Child 1', 'Child 2')
```
3 changes: 2 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ var rules = {
'no-find-dom-node': require('./lib/rules/no-find-dom-node'),
'no-danger-with-children': require('./lib/rules/no-danger-with-children'),
'style-prop-object': require('./lib/rules/style-prop-object'),
'no-unused-prop-types': require('./lib/rules/no-unused-prop-types')
'no-unused-prop-types': require('./lib/rules/no-unused-prop-types'),
'no-children-prop': require('./lib/rules/no-children-prop')
};

var ruleNames = Object.keys(rules);
Expand Down
69 changes: 69 additions & 0 deletions lib/rules/no-children-prop.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* @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: {
description: 'Prevent passing of children as props.',
category: 'Best Practices',
recommended: false
},
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.'
});
}
}
};
}
};
237 changes: 237 additions & 0 deletions tests/lib/rules/no-children-prop.js
Original file line number Diff line number Diff line change
@@ -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: '<div />;',
parserOptions: parserOptions
},
{
code: '<div></div>;',
parserOptions: parserOptions
},
{
code: 'React.createElement("div", {});',
parserOptions: parserOptions
},
{
code: 'React.createElement("div", undefined);',
parserOptions: parserOptions
},
{
code: '<div className="class-name"></div>;',
parserOptions: parserOptions
},
{
code: 'React.createElement("div", {className: "class-name"});',
parserOptions: parserOptions
},
{
code: '<div>Children</div>;',
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: '<div className="class-name">Children</div>;',
parserOptions: parserOptions
},
{
code: 'React.createElement("div", {className: "class-name"}, "Children");',
parserOptions: parserOptions
},
{
code: '<div><div /></div>;',
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: '<div><div /><div /></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: '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: '<MyComponent />',
parserOptions: parserOptions
},
{
code: 'React.createElement(MyComponent);',
parserOptions: parserOptions
},
{
code: 'React.createElement(MyComponent, {});',
parserOptions: parserOptions
},
{
code: 'React.createElement(MyComponent, undefined);',
parserOptions: parserOptions
},
{
code: '<MyComponent>Children</MyComponent>;',
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: '<MyComponent className="class-name"></MyComponent>;',
parserOptions: parserOptions
},
{
code: 'React.createElement(MyComponent, {className: "class-name"});',
parserOptions: parserOptions
},
{
code: '<MyComponent className="class-name">Children</MyComponent>;',
parserOptions: parserOptions
},
{
code: 'React.createElement(MyComponent, {className: "class-name"}, "Children");',
parserOptions: parserOptions
}
],
invalid: [
{
code: '<div children="Children" />;',
errors: [{message: JSX_ERROR}],
parserOptions: parserOptions
},
{
code: '<div children={<div />} />;',
errors: [{message: JSX_ERROR}],
parserOptions: parserOptions
},
{
code: '<div children={[<div />, <div />]} />;',
errors: [{message: JSX_ERROR}],
parserOptions: parserOptions
},
{
code: '<div children="Children">Children</div>;',
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: '<MyComponent children="Children" />',
errors: [{message: JSX_ERROR}],
parserOptions: parserOptions
},
{
code: 'React.createElement(MyComponent, {children: "Children"});',
errors: [{message: CREATE_ELEMENT_ERROR}],
parserOptions: parserOptions
},
{
code: '<MyComponent className="class-name" children="Children" />;',
errors: [{message: JSX_ERROR}],
parserOptions: parserOptions
},
{
code: 'React.createElement(MyComponent, {children: "Children", className: "class-name"});',
errors: [{message: CREATE_ELEMENT_ERROR}],
parserOptions: parserOptions
}
]
});

0 comments on commit 0348173

Please sign in to comment.