Repository: uber-web/baseui
Pull request #241 (2018-09-13 19:18:36)

fix(input): remove default prop for value in BaseInput
Fixes https://github.com/uber-web/baseui/issues/237

nadiia: Added a comment
baseui-probot-app-workflow[bot]: Found TODOs without GitHub issues:
https://github.com/uber-web/baseui/blob/b11e61d85eb566feddbde98f99c9cb40dc29ee9a/src/input/types.js#L50

Patch:
diff --git a/src/input/base-input.js b/src/input/base-input.js
index b32108eec..bd727d985 100644
--- a/src/input/base-input.js
+++ b/src/input/base-input.js
@@ -31,7 +31,6 @@ class BaseInput extends React.Component<BaseInputPropsT, InternalStateT> {
     required: false,
     size: SIZE.default,
     type: 'text',
-    value: '',
   };
 
   state = {
@@ -81,7 +80,7 @@ class BaseInput extends React.Component<BaseInputPropsT, InternalStateT> {
       onBlur: this.onBlur,
       placeholder,
       type,
-      value: value,
+      value,
       ...(type === CUSTOM_INPUT_TYPE.textarea ? {rows} : {}),
     };
   };
diff --git a/src/input/types.js b/src/input/types.js
index 5e7663c78..b7dcd690b 100644
--- a/src/input/types.js
+++ b/src/input/types.js
@@ -47,7 +47,6 @@ export type PropsT = *;
 export type BaseInputComponentsT = {
   InputContainer?: OverrideT<*>,
   Input?: OverrideT<*>,
-  // TODO: Next two seems like shouldn't be in components prop
   Before?: OverrideT<*>,
   After?: OverrideT<*>,
 };
@@ -74,7 +73,7 @@ export type BaseInputPropsT = {
   required: boolean,
   size: SizeT,
   type: string,
-  value: string,
+  value?: string,
   rows?: number,
 };
 


Test patch:
diff --git a/src/input/__tests__/__snapshots__/base-input.test.js.snap b/src/input/__tests__/__snapshots__/base-input.test.js.snap
index 4482a6c13..5d2d7bce1 100644
--- a/src/input/__tests__/__snapshots__/base-input.test.js.snap
+++ b/src/input/__tests__/__snapshots__/base-input.test.js.snap
@@ -36,7 +36,7 @@ Object {
   "placeholder": "Placeholder",
   "styled-component": "true",
   "type": "text",
-  "value": "input value",
+  "value": undefined,
 }
 `;
 
diff --git a/src/input/__tests__/__snapshots__/input.test.js.snap b/src/input/__tests__/__snapshots__/input.test.js.snap
index decfbb419..6cc75fd73 100644
--- a/src/input/__tests__/__snapshots__/input.test.js.snap
+++ b/src/input/__tests__/__snapshots__/input.test.js.snap
@@ -49,7 +49,6 @@ Object {
       onFocus={[Function]}
       placeholder=""
       type="text"
-      value=""
     />,
     null,
   ],
@@ -87,7 +86,6 @@ Object {
       onFocus={[Function]}
       placeholder=""
       type="text"
-      value=""
     />,
     null,
   ],
@@ -125,7 +123,6 @@ Object {
       onFocus={[Function]}
       placeholder=""
       type="text"
-      value=""
     />,
     null,
   ],
diff --git a/src/input/__tests__/base-input.test.js b/src/input/__tests__/base-input.test.js
index 28ed29fb7..f3200c0b7 100644
--- a/src/input/__tests__/base-input.test.js
+++ b/src/input/__tests__/base-input.test.js
@@ -11,7 +11,6 @@ import {BaseInput} from '../index';
 
 test('BaseInput - basic functionality', () => {
   const props = {
-    value: 'input value',
     placeholder: 'Placeholder',
     onFocus: jest.fn(),
     onBlur: jest.fn(),
@@ -68,6 +67,13 @@ test('BaseInput - basic functionality', () => {
   expect(updatedAfter.props()).toMatchSnapshot('After gets correct error prop');
 });
 
+test('BaseInput - should not take default value prop', () => {
+  // $FlowFixMe
+  const wrapper = mount(<BaseInput />);
+  // Guard against passing default value prop
+  expect(wrapper.prop('value')).not.toBeDefined();
+});
+
 test('BaseInput - autoFocus sets the initial focus state', () => {
   const props = {
     autoFocus: true,
diff --git a/src/select/__tests__/__snapshots__/select.test.js.snap b/src/select/__tests__/__snapshots__/select.test.js.snap
index 2549afb9c..1255e9af4 100644
--- a/src/select/__tests__/__snapshots__/select.test.js.snap
+++ b/src/select/__tests__/__snapshots__/select.test.js.snap
@@ -491,7 +491,6 @@ exports[`Stateless select should render component in select mode and false for m
                 required={false}
                 size="default"
                 type="text"
-                value=""
               >
                 <MockStyledComponent
                   $adjoined="none"
@@ -533,7 +532,6 @@ exports[`Stateless select should render component in select mode and false for m
                       onFocus={[Function]}
                       placeholder=""
                       type="text"
-                      value=""
                     >
                       <input
                         aria-invalid={false}
@@ -546,7 +544,6 @@ exports[`Stateless select should render component in select mode and false for m
                         placeholder=""
                         styled-component="true"
                         type="text"
-                        value=""
                       />
                     </MockStyledComponent>
                     <After
@@ -673,7 +670,6 @@ exports[`Stateless select should render component in select mode and true for mu
                 required={false}
                 size="default"
                 type="text"
-                value=""
               >
                 <MockStyledComponent
                   $adjoined="none"
@@ -715,7 +711,6 @@ exports[`Stateless select should render component in select mode and true for mu
                       onFocus={[Function]}
                       placeholder=""
                       type="text"
-                      value=""
                     >
                       <input
                         aria-invalid={false}
@@ -728,7 +723,6 @@ exports[`Stateless select should render component in select mode and true for mu
                         placeholder=""
                         styled-component="true"
                         type="text"
-                        value=""
                       />
                     </MockStyledComponent>
                     <After