From c377e618dcda560de867513292fd64aaec140aec Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sat, 30 Jun 2018 21:20:35 +0200 Subject: [PATCH] [core] Warn about Children.map & Fragment (#12021) --- .../src/SpeedDial/SpeedDial.js | 14 ++++++++++++- .../src/BottomNavigation/BottomNavigation.js | 9 ++++++++ .../src/ExpansionPanel/ExpansionPanel.js | 9 ++++++++ packages/material-ui/src/GridList/GridList.js | 10 +++++++++ packages/material-ui/src/MenuList/MenuList.js | 9 ++++++++ .../material-ui/src/RadioGroup/RadioGroup.js | 9 ++++++++ .../material-ui/src/Select/SelectInput.js | 10 +++++++++ packages/material-ui/src/Step/Step.js | 21 +++++++++++++++---- packages/material-ui/src/Step/Step.test.js | 10 +++++++++ packages/material-ui/src/Tabs/Tabs.js | 8 +++++++ 10 files changed, 104 insertions(+), 5 deletions(-) diff --git a/packages/material-ui-lab/src/SpeedDial/SpeedDial.js b/packages/material-ui-lab/src/SpeedDial/SpeedDial.js index 36378a7651eb98..4d9c2e42b4fe68 100644 --- a/packages/material-ui-lab/src/SpeedDial/SpeedDial.js +++ b/packages/material-ui-lab/src/SpeedDial/SpeedDial.js @@ -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'; @@ -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, { diff --git a/packages/material-ui/src/BottomNavigation/BottomNavigation.js b/packages/material-ui/src/BottomNavigation/BottomNavigation.js index a9fecf3ef884b5..51c625eb79a315 100755 --- a/packages/material-ui/src/BottomNavigation/BottomNavigation.js +++ b/packages/material-ui/src/BottomNavigation/BottomNavigation.js @@ -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 => ({ @@ -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, diff --git a/packages/material-ui/src/ExpansionPanel/ExpansionPanel.js b/packages/material-ui/src/ExpansionPanel/ExpansionPanel.js index d439da8170d18c..ceee29677e2f0b 100644 --- a/packages/material-ui/src/ExpansionPanel/ExpansionPanel.js +++ b/packages/material-ui/src/ExpansionPanel/ExpansionPanel.js @@ -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'; @@ -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, diff --git a/packages/material-ui/src/GridList/GridList.js b/packages/material-ui/src/GridList/GridList.js index e0da6259482442..7618532e87dc7b 100644 --- a/packages/material-ui/src/GridList/GridList.js +++ b/packages/material-ui/src/GridList/GridList.js @@ -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 = { @@ -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; diff --git a/packages/material-ui/src/MenuList/MenuList.js b/packages/material-ui/src/MenuList/MenuList.js index a32ffbf2617150..a5c3882cce648a 100644 --- a/packages/material-ui/src/MenuList/MenuList.js +++ b/packages/material-ui/src/MenuList/MenuList.js @@ -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'; @@ -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 diff --git a/packages/material-ui/src/RadioGroup/RadioGroup.js b/packages/material-ui/src/RadioGroup/RadioGroup.js index 6d96abc9a56650..849d1af01f9239 100644 --- a/packages/material-ui/src/RadioGroup/RadioGroup.js +++ b/packages/material-ui/src/RadioGroup/RadioGroup.js @@ -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'; @@ -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 => { diff --git a/packages/material-ui/src/Select/SelectInput.js b/packages/material-ui/src/Select/SelectInput.js index 1af641ce6509bf..a50eb4d5bc52c6 100644 --- a/packages/material-ui/src/Select/SelectInput.js +++ b/packages/material-ui/src/Select/SelectInput.js @@ -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'; @@ -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) { diff --git a/packages/material-ui/src/Step/Step.js b/packages/material-ui/src/Step/Step.js index fa80b3dda481d9..4b36656c0df491 100644 --- a/packages/material-ui/src/Step/Step.js +++ b/packages/material-ui/src/Step/Step.js @@ -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 => ({ @@ -49,8 +50,20 @@ function Step(props) { return (
- {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, @@ -59,8 +72,8 @@ function Step(props) { last, orientation, ...child.props, - }), - )} + }); + })} {connector && alternativeLabel && !last && diff --git a/packages/material-ui/src/Step/Step.test.js b/packages/material-ui/src/Step/Step.test.js index b8bb93e02b773d..d94a5512c746f2 100644 --- a/packages/material-ui/src/Step/Step.test.js +++ b/packages/material-ui/src/Step/Step.test.js @@ -82,5 +82,15 @@ describe('', () => { const childWrapper = wrapper.find('.hello-world'); assert.strictEqual(childWrapper.props().active, false); }); + + it('should handle invalid children', () => { + const wrapper = shallow( + +

Hello World

+ {null} + , + ); + assert.strictEqual(wrapper.find('.hello-world').length, 1); + }); }); }); diff --git a/packages/material-ui/src/Tabs/Tabs.js b/packages/material-ui/src/Tabs/Tabs.js index 99fbdfa1c8755b..5ccadf20bb7ed6 100644 --- a/packages/material-ui/src/Tabs/Tabs.js +++ b/packages/material-ui/src/Tabs/Tabs.js @@ -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;