Fix Warning: Failed prop type: Invalid prop value supplied to Select (#…

…288) * Fix Warning: Failed prop type: Invalid prop value supplied to Select * Fix stylistic issues - Format code with Prettier - Use double quote for JSX strings - Introduce local defaultValue function * Remove tests that check no error is thrown * Fix defaultValue function * Group Select value PropTypes tests * Move value PropTypes tests in Select component group * Use dot notation
couds · Dec 15, 2020 · 085ae6e · 085ae6e
1 parent 0da8632
commit 085ae6e
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 30 deletions.
diff --git a/src/components/form/components/__test__/__snapshots__/select.test.js.snap b/src/components/form/components/__test__/__snapshots__/select.test.js.snap
@@ -8,7 +8,7 @@ exports[`Select component Should be large, red, disabled and multioption 1`] = `
     disabled={true}
     multiple={true}
     readOnly={false}
-    value=""
+    value={Array []}
   >
     <option>
       1
@@ -98,3 +98,35 @@ exports[`Select component Should use inline styles 1`] = `
   </select>
 </div>
 `;
+
+exports[`Select component value PropTypes Should be multioption and should not accept non array value 1`] = `
+<div
+  className="select is-multiple"
+>
+  <select
+    disabled={false}
+    multiple={true}
+    readOnly={false}
+    value={1}
+  />
+</div>
+`;
+
+exports[`Select component value PropTypes Should not be multioption and should not accept array value 1`] = `
+<div
+  className="select"
+>
+  <select
+    disabled={false}
+    multiple={false}
+    readOnly={false}
+    value={
+      Array [
+        1,
+        2,
+        3,
+      ]
+    }
+  />
+</div>
+`;
diff --git a/src/components/form/components/__test__/select.test.js b/src/components/form/components/__test__/select.test.js
@@ -46,4 +46,20 @@ describe('Select component', () => {
     );
     expect(component.toJSON()).toMatchSnapshot();
   });
+  describe('value PropTypes', () => {
+    it('Should be multioption and should not accept non array value', () => {
+      const spy = jest.spyOn(console, 'error').mockImplementation();
+      const component = renderer.create(<Select multiple value={1}></Select>);
+      expect(component.toJSON()).toMatchSnapshot();
+      expect(spy).toBeCalled();
+      spy.mockRestore();
+    });
+    it('Should not be multioption and should not accept array value', () => {
+      const spy = jest.spyOn(console, 'error').mockImplementation();
+      const component = renderer.create(<Select value={[1, 2, 3]}></Select>);
+      expect(component.toJSON()).toMatchSnapshot();
+      expect(spy).toBeCalled();
+      spy.mockRestore();
+    });
+  });
 });
diff --git a/src/components/form/components/select.js b/src/components/form/components/select.js
@@ -21,30 +21,39 @@ const Select = ({
   name,
   domRef,
   ...props
-}) => (
-  <Element
-    domRef={domRef}
-    className={classnames('select', className, {
-      [`is-${size}`]: size,
-      [`is-${color}`]: color,
-      'is-loading': loading,
-      'is-multiple': multiple,
-    })}
-    style={style}
-  >
+}) => {
+  /**
+   * Return default value for value prop
+   */
+  function defaultValue() {
+    return multiple ? [] : '';
+  }
+
+  return (
     <Element
-      renderAs="select"
-      {...props}
-      multiple={multiple}
-      value={value}
-      readOnly={readOnly}
-      disabled={disabled}
-      name={name}
+      domRef={domRef}
+      className={classnames('select', className, {
+        [`is-${size}`]: size,
+        [`is-${color}`]: color,
+        'is-loading': loading,
+        'is-multiple': multiple,
+      })}
+      style={style}
     >
-      {children}
+      <Element
+        renderAs="select"
+        {...props}
+        multiple={multiple}
+        value={value !== undefined ? value : defaultValue()}
+        readOnly={readOnly}
+        disabled={disabled}
+        name={name}
+      >
+        {children}
+      </Element>
     </Element>
-  </Element>
-);
+  );
+};
 
 Select.propTypes = {
   ...modifiers.propTypes,
@@ -57,13 +66,29 @@ Select.propTypes = {
   disabled: PropTypes.bool,
   multiple: PropTypes.bool,
   loading: PropTypes.bool,
-  value: PropTypes.oneOfType([
-    PropTypes.string,
-    PropTypes.number,
-    PropTypes.arrayOf(
-      PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
-    ),
-  ]),
+  value: function (props, propName, componentName) {
+    if (props.multiple) {
+      PropTypes.checkPropTypes(
+        {
+          [propName]: PropTypes.arrayOf(
+            PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
+          ),
+        },
+        props,
+        propName,
+        componentName,
+      );
+    } else {
+      PropTypes.checkPropTypes(
+        {
+          [propName]: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
+        },
+        props,
+        propName,
+        componentName,
+      );
+    }
+  },
   /**
    * The name of the input field Commonly used for [multi-input handling](https://reactjs.org/docs/forms.html#handling-multiple-inputs)
    */
@@ -74,7 +99,7 @@ Select.defaultProps = {
   ...modifiers.defaultProps,
   children: null,
   className: undefined,
-  value: '',
+  value: undefined,
   style: undefined,
   size: undefined,
   color: undefined,