Skip to content

Commit

Permalink
[core] Warn about Children.map & Fragment (#12021)
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari authored Jun 30, 2018
1 parent a2eb975 commit c377e61
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 5 deletions.
14 changes: 13 additions & 1 deletion packages/material-ui-lab/src/SpeedDial/SpeedDial.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
import classNames from 'classnames';
import ReactDOM from 'react-dom';
import keycode from 'keycode';
import warning from 'warning';
import { withStyles } from '@material-ui/core/styles';
import Zoom from '@material-ui/core/Zoom';
import { duration } from '@material-ui/core/styles/transitions';
Expand Down Expand Up @@ -114,7 +115,18 @@ class SpeedDial extends React.Component {

let validChildCount = 0;
const children = React.Children.map(childrenProp, child => {
if (!React.isValidElement(child)) return null;
if (!React.isValidElement(child)) {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the SpeedDial component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

const delay = 30 * (open ? validChildCount : totalValidChildren - validChildCount);
validChildCount += 1;
return React.cloneElement(child, {
Expand Down
9 changes: 9 additions & 0 deletions packages/material-ui/src/BottomNavigation/BottomNavigation.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import warning from 'warning';
import withStyles from '../styles/withStyles';

export const styles = theme => ({
Expand Down Expand Up @@ -30,6 +31,14 @@ function BottomNavigation(props) {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the BottomNavigation component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

const childValue = child.props.value === undefined ? childIndex : child.props.value;
return React.cloneElement(child, {
selected: childValue === value,
Expand Down
9 changes: 9 additions & 0 deletions packages/material-ui/src/ExpansionPanel/ExpansionPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import warning from 'warning';
import Collapse from '../Collapse';
import Paper from '../Paper';
import withStyles from '../styles/withStyles';
Expand Down Expand Up @@ -133,6 +134,14 @@ class ExpansionPanel extends React.Component {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the ExpansionPanel component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

if (isMuiElement(child, ['ExpansionPanelSummary'])) {
summary = React.cloneElement(child, {
disabled,
Expand Down
10 changes: 10 additions & 0 deletions packages/material-ui/src/GridList/GridList.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import warning from 'warning';
import withStyles from '../styles/withStyles';

export const styles = {
Expand Down Expand Up @@ -37,6 +38,15 @@ function GridList(props) {
if (!React.isValidElement(child)) {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the GridList component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

const childCols = child.props.cols || 1;
const childRows = child.props.rows || 1;

Expand Down
9 changes: 9 additions & 0 deletions packages/material-ui/src/MenuList/MenuList.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import ReactDOM from 'react-dom';
import keycode from 'keycode';
import warning from 'warning';
import ownerDocument from '../utils/ownerDocument';
import List from '../List';

Expand Down Expand Up @@ -144,6 +145,14 @@ class MenuList extends React.Component {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the MenuList component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

return React.cloneElement(child, {
tabIndex: index === this.state.currentTabIndex ? 0 : -1,
ref: child.props.selected
Expand Down
9 changes: 9 additions & 0 deletions packages/material-ui/src/RadioGroup/RadioGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import React from 'react';
import PropTypes from 'prop-types';
import warning from 'warning';
import FormGroup from '../FormGroup';
import { createChainedFunction, find } from '../utils/helpers';

Expand Down Expand Up @@ -47,6 +48,14 @@ class RadioGroup extends React.Component {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the RadioGroup component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

return React.cloneElement(child, {
name,
inputRef: node => {
Expand Down
10 changes: 10 additions & 0 deletions packages/material-ui/src/Select/SelectInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import keycode from 'keycode';
import warning from 'warning';
import Menu from '../Menu/Menu';
import { isFilled } from '../Input/Input';

Expand Down Expand Up @@ -207,6 +208,15 @@ class SelectInput extends React.Component {
if (!React.isValidElement(child)) {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the Select component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

let selected;

if (multiple) {
Expand Down
21 changes: 17 additions & 4 deletions packages/material-ui/src/Step/Step.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import warning from 'warning';
import withStyles from '../styles/withStyles';

export const styles = theme => ({
Expand Down Expand Up @@ -49,8 +50,20 @@ function Step(props) {

return (
<div className={className} {...other}>
{React.Children.map(children, child =>
React.cloneElement(child, {
{React.Children.map(children, child => {
if (!React.isValidElement(child)) {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the Step component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

return React.cloneElement(child, {
active,
alternativeLabel,
completed,
Expand All @@ -59,8 +72,8 @@ function Step(props) {
last,
orientation,
...child.props,
}),
)}
});
})}
{connector &&
alternativeLabel &&
!last &&
Expand Down
10 changes: 10 additions & 0 deletions packages/material-ui/src/Step/Step.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,15 @@ describe('<Step />', () => {
const childWrapper = wrapper.find('.hello-world');
assert.strictEqual(childWrapper.props().active, false);
});

it('should handle invalid children', () => {
const wrapper = shallow(
<Step label="Step One" index={1} orientation="horizontal">
<h1 className="hello-world">Hello World</h1>
{null}
</Step>,
);
assert.strictEqual(wrapper.find('.hello-world').length, 1);
});
});
});
8 changes: 8 additions & 0 deletions packages/material-ui/src/Tabs/Tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,14 @@ class Tabs extends React.Component {
return null;
}

warning(
child.type !== React.Fragment,
[
"Material-UI: the Tabs component doesn't accept a Fragment as a child.",
'Consider providing an array instead.',
].join('\n'),
);

const childValue = child.props.value === undefined ? childIndex : child.props.value;
this.valueToIndex.set(childValue, childIndex);
const selected = childValue === value;
Expand Down

0 comments on commit c377e61

Please sign in to comment.