Skip to content

Commit

Permalink
Add new no-children-prop rule (Fixes #720)
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
  • Loading branch information
benstepp committed Jul 27, 2016
1 parent 8b8eba7 commit b1f9f08
Show file tree
Hide file tree
Showing 5 changed files with 343 additions and 1 deletion.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -160,6 +161,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-children-prop](docs/rules/no-children-prop.md)
* [react/no-danger](docs/rules/no-danger.md)
* [react/no-deprecated](docs/rules/no-deprecated.md)
* [react/no-did-mount-set-state](docs/rules/no-did-mount-set-state.md) with `allow-in-func` option
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')
```
4 changes: 3 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -74,6 +75,7 @@ module.exports = {
'react/jsx-no-undef': 2,
'react/jsx-uses-react': 2,
'react/jsx-uses-vars': 2,
'react/no-children-prop': 2,
'react/no-danger': 2,
'react/no-deprecated': 2,
'react/no-did-mount-set-state': [2, 'allow-in-func'],
Expand Down
65 changes: 65 additions & 0 deletions lib/rules/no-children-prop.js
Original file line number Diff line number Diff line change
@@ -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.'
});
}
}
};
}
};
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 b1f9f08

Please sign in to comment.