Corrects error message for select with props.multiple set to true and…

… a null value.
facebook · Oct 27, 2017 · 5e90c3e · 5e90c3e
1 parent cc54b6f
commit 5e90c3e
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 7 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js
@@ -796,7 +796,7 @@ describe('ReactDOMInput', () => {
     ReactTestUtils.renderIntoDocument(<input type="text" value={null} />);
     expectDev(console.error.calls.argsFor(0)[0]).toContain(
       '`value` prop on `input` should not be null. ' +
-        'Consider using the empty string to clear the component or `undefined` ' +
+        'Consider using an empty string to clear the component or `undefined` ' +
         'for uncontrolled components.',
     );
 

diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js
@@ -514,7 +514,7 @@ describe('ReactDOMSelect', () => {
     );
     expectDev(console.error.calls.argsFor(0)[0]).toContain(
       '`value` prop on `select` should not be null. ' +
-        'Consider using the empty string to clear the component or `undefined` ' +
+        'Consider using an empty string to clear the component or `undefined` ' +
         'for uncontrolled components.',
     );
 
@@ -524,6 +524,25 @@ describe('ReactDOMSelect', () => {
     expectDev(console.error.calls.count()).toBe(1);
   });
 
+  it('should warn if value is null and multiple is true', () => {
+    spyOn(console, 'error');
+    ReactTestUtils.renderIntoDocument(
+      <select value={null} multiple={true}><option value="test" /></select>,
+    );
+
+    expectDev(console.error.calls.argsFor(0)[0]).toContain(
+      '`value` prop on `select` should not be null. ' +
+        'Consider using an empty array when `multiple` is ' +
+        'set to `true` to clear the component or `undefined` ' +
+        'for uncontrolled components.',
+    );
+
+    ReactTestUtils.renderIntoDocument(
+      <select value={null} multiple={true}><option value="test" /></select>,
+    );
+    expectDev(console.error.calls.count()).toBe(1);
+  });
+
   it('should refresh state on change', () => {
     var stub = (
       <select value="giraffe" onChange={noop}>

diff --git a/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js b/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js
@@ -357,7 +357,7 @@ describe('ReactDOMTextarea', () => {
     ReactTestUtils.renderIntoDocument(<textarea value={null} />);
     expectDev(console.error.calls.argsFor(0)[0]).toContain(
       '`value` prop on `textarea` should not be null. ' +
-        'Consider using the empty string to clear the component or `undefined` ' +
+        'Consider using an empty string to clear the component or `undefined` ' +
         'for uncontrolled components.',
     );
 

diff --git a/packages/react-dom/src/shared/ReactDOMNullInputValuePropHook.js b/packages/react-dom/src/shared/ReactDOMNullInputValuePropHook.js
@@ -12,7 +12,10 @@ if (__DEV__) {
   var warning = require('fbjs/lib/warning');
 }
 
-var didWarnValueNull = false;
+var didWarnValueNull = {
+  default: false,
+  multiple: false,
+};
 
 function getStackAddendum() {
   var stack = ReactDebugCurrentFrame.getStackAddendum();
@@ -23,17 +26,25 @@ function validateProperties(type, props) {
   if (type !== 'input' && type !== 'textarea' && type !== 'select') {
     return;
   }
-  if (props != null && props.value === null && !didWarnValueNull) {
+
+  var isMultipleSelect = type === 'select' && props.multiple === true;
+  var errorType = isMultipleSelect ? 'multiple' : 'default';
+  var isFirstError = !didWarnValueNull[errorType];
+
+  if (props != null && props.value === null && isFirstError) {
     warning(
       false,
       '`value` prop on `%s` should not be null. ' +
-        'Consider using the empty string to clear the component or `undefined` ' +
+        'Consider using an empty %s to clear the component or `undefined` ' +
         'for uncontrolled components.%s',
       type,
+      errorType === 'multiple'
+        ? 'array when multiple is set to true'
+        : 'string',
       getStackAddendum(),
     );
 
-    didWarnValueNull = true;
+    didWarnValueNull[errorType] = true;
   }
 }