Changed the error message displayed when a select element has props.m…

…ultiple set to true and value set to null (#11141) * Corrects error message for select with props.multiple set to true and a null value. * Don't bother deduplicating based on type * Make the code a bit simpler (and more verbose)
facebook · Oct 31, 2017 · 4a43cf6 · 4a43cf6
1 parent 4ce5da7
commit 4a43cf6
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 12 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
@@ -18,6 +18,7 @@ describe('ReactDOMSelect', () => {
   var noop = function() {};
 
   beforeEach(() => {
+    jest.resetModules();
     React = require('react');
     ReactDOM = require('react-dom');
     ReactDOMServer = require('react-dom/server');
@@ -514,7 +515,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 +525,26 @@ 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.count()).toBe(1);
+    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
@@ -23,17 +23,28 @@ function validateProperties(type, props) {
   if (type !== 'input' && type !== 'textarea' && type !== 'select') {
     return;
   }
-  if (props != null && props.value === null && !didWarnValueNull) {
-    warning(
-      false,
-      '`value` prop on `%s` should not be null. ' +
-        'Consider using the empty string to clear the component or `undefined` ' +
-        'for uncontrolled components.%s',
-      type,
-      getStackAddendum(),
-    );
 
+  if (props != null && props.value === null && !didWarnValueNull) {
     didWarnValueNull = true;
+    if (type === 'select' && props.multiple) {
+      warning(
+        false,
+        '`value` prop on `%s` should not be null. ' +
+          'Consider using an empty array when `multiple` is set to `true` ' +
+          'to clear the component or `undefined` for uncontrolled components.%s',
+        type,
+        getStackAddendum(),
+      );
+    } else {
+      warning(
+        false,
+        '`value` prop on `%s` should not be null. ' +
+          'Consider using an empty string to clear the component or `undefined` ' +
+          'for uncontrolled components.%s',
+        type,
+        getStackAddendum(),
+      );
+    }
   }
 }