Changed the error message displayed when a select element has props.multiple 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)
Neal Wright committed
Oct 31, 2017 at 04:47 UTC
4a43cf6eac39e95be30c86fc0de8a62bc132c4b0
4 files changed
+44
-12
packages/react-dom/src/__tests__/ReactDOMInput-test.js
+1
-1
@@ -796,7 +796,7 @@ describe('ReactDOMInput', () => {
796
ReactTestUtils.renderIntoDocument(<input type="text" value={null} />);
797
expectDev(console.error.calls.argsFor(0)[0]).toContain(
798
'`value` prop on `input` should not be null. ' +
799
- 'Consider using the empty string to clear the component or `undefined` ' +
799
+ 'Consider using an empty string to clear the component or `undefined` ' +
800
'for uncontrolled components.',
801
);
802
packages/react-dom/src/__tests__/ReactDOMSelect-test.js
+22
-1
@@ -18,6 +18,7 @@ describe('ReactDOMSelect', () => {
18
var noop = function() {};
19
20
beforeEach(() => {
21
+ jest.resetModules();
22
React = require('react');
23
ReactDOM = require('react-dom');
24
ReactDOMServer = require('react-dom/server');
@@ -514,7 +515,7 @@ describe('ReactDOMSelect', () => {
515
);
516
expectDev(console.error.calls.argsFor(0)[0]).toContain(
517
'`value` prop on `select` should not be null. ' +
517
- 'Consider using the empty string to clear the component or `undefined` ' +
518
+ 'Consider using an empty string to clear the component or `undefined` ' +
519
'for uncontrolled components.',
520
);
521
@@ -524,6 +525,26 @@ describe('ReactDOMSelect', () => {
525
expectDev(console.error.calls.count()).toBe(1);
526
});
527
528
+ it('should warn if value is null and multiple is true', () => {
529
+ spyOn(console, 'error');
530
+ ReactTestUtils.renderIntoDocument(
531
+ <select value={null} multiple={true}><option value="test" /></select>,
532
+ );
533
+
534
+ expectDev(console.error.calls.count()).toBe(1);
535
+ expectDev(console.error.calls.argsFor(0)[0]).toContain(
536
+ '`value` prop on `select` should not be null. ' +
537
+ 'Consider using an empty array when `multiple` is ' +
538
+ 'set to `true` to clear the component or `undefined` ' +
539
+ 'for uncontrolled components.',
540
+ );
541
+
542
+ ReactTestUtils.renderIntoDocument(
543
+ <select value={null} multiple={true}><option value="test" /></select>,
544
+ );
545
+ expectDev(console.error.calls.count()).toBe(1);
546
+ });
547
+
548
it('should refresh state on change', () => {
549
var stub = (
550
<select value="giraffe" onChange={noop}>
packages/react-dom/src/__tests__/ReactDOMTextarea-test.js
+1
-1
@@ -357,7 +357,7 @@ describe('ReactDOMTextarea', () => {
357
ReactTestUtils.renderIntoDocument(<textarea value={null} />);
358
expectDev(console.error.calls.argsFor(0)[0]).toContain(
359
'`value` prop on `textarea` should not be null. ' +
360
- 'Consider using the empty string to clear the component or `undefined` ' +
360
+ 'Consider using an empty string to clear the component or `undefined` ' +
361
'for uncontrolled components.',
362
);
363
packages/react-dom/src/shared/ReactDOMNullInputValuePropHook.js
+20
-9
@@ -23,17 +23,28 @@ function validateProperties(type, props) {
23
if (type !== 'input' && type !== 'textarea' && type !== 'select') {
24
return;
25
}
26
- if (props != null && props.value === null && !didWarnValueNull) {
27
- warning(
28
- false,
29
- '`value` prop on `%s` should not be null. ' +
30
- 'Consider using the empty string to clear the component or `undefined` ' +
31
- 'for uncontrolled components.%s',
32
- type,
33
- getStackAddendum(),
34
- );
26
27
+ if (props != null && props.value === null && !didWarnValueNull) {
28
didWarnValueNull = true;
29
+ if (type === 'select' && props.multiple) {
30
+ warning(
31
+ false,
32
+ '`value` prop on `%s` should not be null. ' +
33
+ 'Consider using an empty array when `multiple` is set to `true` ' +
34
+ 'to clear the component or `undefined` for uncontrolled components.%s',
35
+ type,
36
+ getStackAddendum(),
37
+ );
38
+ } else {
39
+ warning(
40
+ false,
41
+ '`value` prop on `%s` should not be null. ' +
42
+ 'Consider using an empty string to clear the component or `undefined` ' +
43
+ 'for uncontrolled components.%s',
44
+ type,
45
+ getStackAddendum(),
46
+ );
47
+ }
48
}
49
}
50