[core] Warn about Children.map & Fragment (mui#12021)

acroyear · Jul 14, 2018 · 26a8a9d · 26a8a9d
1 parent a4334b8
commit 26a8a9d
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 (
     <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,
@@ -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
@@ -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);
+    });
   });
 });
diff --git 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;