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

… a null value.
facebook · Oct 7, 2017 · 53e7f7e · 53e7f7e
1 parent 4131af3
commit 53e7f7e
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 72 deletions.
diff --git a/scripts/rollup/results.json b/scripts/rollup/results.json
@@ -1,8 +1,8 @@
 {
   "bundleSizes": {
     "react.development.js (UMD_DEV)": {
-      "size": 54777,
-      "gzip": 14345
+      "size": 54819,
+      "gzip": 14348
     },
     "react.production.min.js (UMD_PROD)": {
       "size": 6634,
@@ -25,40 +25,40 @@
       "gzip": 6703
     },
     "react-dom.development.js (UMD_DEV)": {
-      "size": 614249,
-      "gzip": 141958
+      "size": 622093,
+      "gzip": 143491
     },
     "react-dom.production.min.js (UMD_PROD)": {
-      "size": 99500,
-      "gzip": 31459
+      "size": 100052,
+      "gzip": 31613
     },
     "react-dom.development.js (NODE_DEV)": {
-      "size": 576510,
-      "gzip": 133279
+      "size": 584378,
+      "gzip": 134811
     },
     "react-dom.production.min.js (NODE_PROD)": {
-      "size": 104453,
-      "gzip": 32832
+      "size": 104973,
+      "gzip": 32985
     },
     "ReactDOMFiber-dev.js (FB_DEV)": {
-      "size": 573391,
-      "gzip": 132712
+      "size": 581259,
+      "gzip": 134242
     },
     "ReactDOMFiber-prod.js (FB_PROD)": {
-      "size": 409575,
-      "gzip": 91600
+      "size": 412354,
+      "gzip": 92115
     },
     "react-dom-test-utils.development.js (NODE_DEV)": {
-      "size": 41660,
-      "gzip": 11093
+      "size": 41687,
+      "gzip": 11100
     },
     "ReactTestUtils-dev.js (FB_DEV)": {
-      "size": 41222,
-      "gzip": 11028
+      "size": 41249,
+      "gzip": 11034
     },
     "react-dom-unstable-native-dependencies.development.js (UMD_DEV)": {
-      "size": 86507,
-      "gzip": 21920
+      "size": 86535,
+      "gzip": 21924
     },
     "react-dom-unstable-native-dependencies.production.min.js (UMD_PROD)": {
       "size": 15163,
@@ -81,76 +81,76 @@
       "gzip": 15465
     },
     "react-dom-server.browser.development.js (UMD_DEV)": {
-      "size": 129604,
-      "gzip": 33468
+      "size": 130439,
+      "gzip": 33699
     },
     "react-dom-server.browser.production.min.js (UMD_PROD)": {
-      "size": 14959,
-      "gzip": 5848
+      "size": 15059,
+      "gzip": 5885
     },
     "react-dom-server.browser.development.js (NODE_DEV)": {
-      "size": 99526,
-      "gzip": 26362
+      "size": 100349,
+      "gzip": 26599
     },
     "react-dom-server.browser.production.min.js (NODE_PROD)": {
-      "size": 14889,
-      "gzip": 5844
+      "size": 14989,
+      "gzip": 5873
     },
     "ReactDOMServer-dev.js (FB_DEV)": {
-      "size": 98947,
-      "gzip": 26272
+      "size": 99770,
+      "gzip": 26499
     },
     "ReactDOMServer-prod.js (FB_PROD)": {
-      "size": 42275,
-      "gzip": 11813
+      "size": 42468,
+      "gzip": 11846
     },
     "react-dom-server.node.development.js (NODE_DEV)": {
-      "size": 101804,
-      "gzip": 26916
+      "size": 102627,
+      "gzip": 27151
     },
     "react-dom-server.node.production.min.js (NODE_PROD)": {
-      "size": 15814,
-      "gzip": 6172
+      "size": 15914,
+      "gzip": 6203
     },
     "react-art.development.js (UMD_DEV)": {
-      "size": 366731,
-      "gzip": 81411
+      "size": 304065,
+      "gzip": 65801
     },
     "react-art.production.min.js (UMD_PROD)": {
-      "size": 82271,
-      "gzip": 25526
+      "size": 46364,
+      "gzip": 14489
     },
     "react-art.development.js (NODE_DEV)": {
-      "size": 291038,
-      "gzip": 62178
+      "size": 294139,
+      "gzip": 62704
     },
     "react-art.production.min.js (NODE_PROD)": {
-      "size": 52180,
-      "gzip": 16354
+      "size": 52306,
+      "gzip": 16377
     },
     "ReactARTFiber-dev.js (FB_DEV)": {
-      "size": 289892,
-      "gzip": 62221
+      "size": 292993,
+      "gzip": 62753
     },
     "ReactARTFiber-prod.js (FB_PROD)": {
-      "size": 217580,
-      "gzip": 45263
+      "size": 218059,
+      "gzip": 45323
     },
     "ReactNativeFiber-dev.js (RN_DEV)": {
-      "size": 275060,
-      "gzip": 48092
+      "size": 277048,
+      "gzip": 48329
     },
     "ReactNativeFiber-prod.js (RN_PROD)": {
-      "size": 215733,
-      "gzip": 37458
+      "size": 216111,
+      "gzip": 37521
     },
     "react-test-renderer.development.js (NODE_DEV)": {
-      "size": 295122,
-      "gzip": 62617
+      "size": 298257,
+      "gzip": 63139
     },
     "ReactTestRendererFiber-dev.js (FB_DEV)": {
-      "size": 293966,
-      "gzip": 62666
+      "size": 297101,
+      "gzip": 63194
     },
     "react-test-renderer-shallow.development.js (NODE_DEV)": {
       "size": 9157,
@@ -161,8 +161,8 @@
       "gzip": 2208
     },
     "react-noop-renderer.development.js (NODE_DEV)": {
-      "size": 282598,
-      "gzip": 59528
+      "size": 285699,
+      "gzip": 60046
     },
     "react-dom-server.development.js (UMD_DEV)": {
       "size": 120897,
@@ -189,16 +189,16 @@
       "gzip": 7520
     },
     "ReactNativeRTFiber-dev.js (RN_DEV)": {
-      "size": 218534,
-      "gzip": 37944
+      "size": 209321,
+      "gzip": 35877
     },
     "ReactNativeRTFiber-prod.js (RN_PROD)": {
-      "size": 167912,
-      "gzip": 28774
+      "size": 158131,
+      "gzip": 26596
     },
     "react-test-renderer.production.min.js (NODE_PROD)": {
-      "size": 53838,
-      "gzip": 16625
+      "size": 53964,
+      "gzip": 16647
     },
     "react-test-renderer-shallow.production.min.js (NODE_PROD)": {
       "size": 4508,

diff --git a/src/renderers/dom/shared/hooks/ReactDOMNullInputValuePropHook.js b/src/renderers/dom/shared/hooks/ReactDOMNullInputValuePropHook.js
@@ -14,7 +14,10 @@ if (__DEV__) {
   var {ReactDebugCurrentFrame} = require('ReactGlobalSharedState');
 }
 
-var didWarnValueNull = false;
+var didWarnValueNull = {
+  default: false,
+  multiple: false,
+};
 
 function getStackAddendum() {
   var stack = ReactDebugCurrentFrame.getStackAddendum();
@@ -25,17 +28,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;
   }
 }
 

diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
@@ -810,7 +810,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/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js
@@ -508,13 +508,13 @@ describe('ReactDOMSelect', () => {
 
   it('should warn if value is null', () => {
     spyOn(console, 'error');
-
     ReactTestUtils.renderIntoDocument(
       <select value={null}><option value="test" /></select>,
     );
+
     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}>
@@ -556,6 +575,7 @@ describe('ReactDOMSelect', () => {
         'element and remove one of these props. More info: ' +
         'https://fb.me/react-controlled-components',
     );
+    expectDev(console.error.calls.count()).toBe(1);
 
     ReactTestUtils.renderIntoDocument(
       <select value="giraffe" defaultValue="giraffe" readOnly={true}>

diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js b/src/renderers/dom/shared/wrappers/__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.',
     );