fix(Icon): add prop type check for either icon or name prop exists (#1132)

* fix(Icon): add prop type check for either icon or name prop exists

* fix(Icon): add better possible cause in the invariant()

* chore(Icon): move the file for prop type check

* chore(Icon): make prop type check API simpler

Author: asudoh

src/components/DataTable/TableToolbarAction.js

@@ -2,6 +2,7 @@ import cx from 'classnames';
 import PropTypes from 'prop-types';
 import React from 'react';
 import Icon from '../Icon';
+import isRequiredOneOf from '../../prop-types/isRequiredOneOf';
 
 const TableToolbarAction = ({
   className,
@@ -27,20 +28,22 @@ TableToolbarAction.propTypes = {
   children: PropTypes.node,
   className: PropTypes.string,
 
-  /**
-   * Specify the icon for the toolbar action
-   */
-  icon: PropTypes.shape({
-    width: PropTypes.string,
-    height: PropTypes.string,
-    viewBox: PropTypes.string.isRequired,
-    svgData: PropTypes.object.isRequired,
-  }).isRequired,
+  ...isRequiredOneOf({
+    /**
+     * Specify the icon for the toolbar action
+     */
+    icon: PropTypes.shape({
+      width: PropTypes.string,
+      height: PropTypes.string,
+      viewBox: PropTypes.string.isRequired,
+      svgData: PropTypes.object.isRequired,
+    }),
 
-  /**
-   * Specify the name of the icon for the toolbar action
-   */
-  iconName: PropTypes.string,
+    /**
+     * Specify the name of the icon for the toolbar action
+     */
+    iconName: PropTypes.string,
+  }),
 
   /**
    * Specify the description of the icon for the toolbar action

src/components/Icon/Icon-test.js

@@ -45,10 +45,6 @@ describe('Icon', () => {
     it('should recieve style props', () => {
       expect(wrapper.props().style).toEqual({ transition: '2s' });
     });
-
-    it('should not crash when there is no name prop', () => {
-      mount(<Icon />);
-    });
   });
 
   describe('Supports legacy icon', () => {

src/components/Icon/Icon.js

@@ -1,6 +1,8 @@
+import invariant from 'invariant';
 import PropTypes from 'prop-types';
 import React from 'react';
 import icons from 'carbon-icons';
+import isRequiredOneOf from '../../prop-types/isRequiredOneOf';
 
 /**
  * The icons list object from `carbon-icons`.
@@ -88,7 +90,15 @@ export function svgShapes(svgData) {
 }
 
 export function isPrefixed(name) {
-  return !!name && name.split('--')[0] === 'icon';
+  if (__DEV__) {
+    invariant(
+      typeof name === 'string',
+      '[Icon] icon name is missing. You likely forgot to specify the icon, ' +
+        'or are using older (pre-`7.x`) version of `carbon-icons` library. ' +
+        'To specify the icon, use either `icon` (data) or `name` (icon name) properties.'
+    );
+  }
+  return name && name.split('--')[0] === 'icon';
 }
 
 const Icon = ({
@@ -155,21 +165,23 @@ Icon.propTypes = {
    */
   height: PropTypes.string,
 
-  /**
-   * The icon data.
-   */
-  icon: PropTypes.shape({
-    width: PropTypes.string,
-    height: PropTypes.string,
-    viewBox: PropTypes.string.isRequired,
-    svgData: PropTypes.object.isRequired,
+  ...isRequiredOneOf({
+    /**
+     * The icon data.
+     */
+    icon: PropTypes.shape({
+      width: PropTypes.string,
+      height: PropTypes.string,
+      viewBox: PropTypes.string.isRequired,
+      svgData: PropTypes.object.isRequired,
+    }),
+
+    /**
+     * The name in the sprite.
+     */
+    name: PropTypes.string,
   }),
 
-  /**
-   * The name in the sprite.
-   */
-  name: PropTypes.string,
-
   /**
    * The `role` attribute.
    */

src/prop-types/isRequiredOneOf.js

@@ -0,0 +1,26 @@
+/**
+ * @param {Object<string, Function>} propTypes The list of type checkers, keyed by prop names.
+ * @returns {Object<string, Function>}
+ *   The new prop type checkers that checks if one of the given props exist,
+ *   in addition to the original type checkings.
+ */
+export default function isRequiredOneOf(propTypes) {
+  const names = Object.keys(propTypes);
+  const checker = propType => (props, propName, componentName, ...rest) => {
+    if (__DEV__ && names.every(name => !props.hasOwnProperty(name))) {
+      return new Error(
+        `${componentName} requires one of the following props: ${names.join(
+          ', '
+        )}`
+      );
+    }
+    return propType(props, propName, componentName, ...rest);
+  };
+  return names.reduce(
+    (o, name) => ({
+      ...o,
+      [name]: checker(propTypes[name]),
+    }),
+    {}
+  );
+}