Skip to content

Commit 151b778

Browse files
nealwrightEthan-Arrowood
authored andcommitted
Changed the error message displayed when a select element has props.multiple set to true and value set to null (facebook#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)
1 parent e11dc40 commit 151b778

File tree

4 files changed

+44
-12
lines changed

4 files changed

+44
-12
lines changed

packages/react-dom/src/__tests__/ReactDOMInput-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ describe('ReactDOMInput', () => {
796796
ReactTestUtils.renderIntoDocument(<input type="text" value={null} />);
797797
expectDev(console.error.calls.argsFor(0)[0]).toContain(
798798
'`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` ' +
800800
'for uncontrolled components.',
801801
);
802802

packages/react-dom/src/__tests__/ReactDOMSelect-test.js

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ describe('ReactDOMSelect', () => {
1818
var noop = function() {};
1919

2020
beforeEach(() => {
21+
jest.resetModules();
2122
React = require('react');
2223
ReactDOM = require('react-dom');
2324
ReactDOMServer = require('react-dom/server');
@@ -514,7 +515,7 @@ describe('ReactDOMSelect', () => {
514515
);
515516
expectDev(console.error.calls.argsFor(0)[0]).toContain(
516517
'`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` ' +
518519
'for uncontrolled components.',
519520
);
520521

@@ -524,6 +525,26 @@ describe('ReactDOMSelect', () => {
524525
expectDev(console.error.calls.count()).toBe(1);
525526
});
526527

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+
527548
it('should refresh state on change', () => {
528549
var stub = (
529550
<select value="giraffe" onChange={noop}>

packages/react-dom/src/__tests__/ReactDOMTextarea-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ describe('ReactDOMTextarea', () => {
357357
ReactTestUtils.renderIntoDocument(<textarea value={null} />);
358358
expectDev(console.error.calls.argsFor(0)[0]).toContain(
359359
'`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` ' +
361361
'for uncontrolled components.',
362362
);
363363

packages/react-dom/src/shared/ReactDOMNullInputValuePropHook.js

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,28 @@ function validateProperties(type, props) {
2323
if (type !== 'input' && type !== 'textarea' && type !== 'select') {
2424
return;
2525
}
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-
);
3526

27+
if (props != null && props.value === null && !didWarnValueNull) {
3628
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+
}
3748
}
3849
}
3950

0 commit comments

Comments
 (0)