Allow complex objects as children of option only if value is provided (#21431)
Sebastian Markbåge committed
May 5, 2021 at 11:40 UTC
8ea11306ad473b26a2c899ef7a893d25413f3510
7 files changed
+164
-101
packages/react-dom/src/__tests__/ReactDOMOption-test.js
+96
-19
@@ -12,12 +12,14 @@
12
describe('ReactDOMOption', () => {
13
let React;
14
let ReactDOM;
15
+ let ReactDOMServer;
16
let ReactTestUtils;
17
18
beforeEach(() => {
19
jest.resetModules();
20
React = require('react');
21
ReactDOM = require('react-dom');
22
+ ReactDOMServer = require('react-dom/server');
23
ReactTestUtils = require('react-dom/test-utils');
24
});
25
@@ -32,9 +34,9 @@ describe('ReactDOMOption', () => {
34
expect(node.innerHTML).toBe('1 foo');
35
});
36
35
- it('should ignore and warn invalid children types', () => {
37
+ it('should warn for invalid child tags', () => {
38
const el = (
37
- <option>
39
+ <option value="12">
40
{1} <div /> {2}
41
</option>
42
);
@@ -42,10 +44,45 @@ describe('ReactDOMOption', () => {
44
expect(() => {
45
node = ReactTestUtils.renderIntoDocument(el);
46
}).toErrorDev(
45
- 'Only strings and numbers are supported as <option> children.\n' +
47
+ 'validateDOMNesting(...): <div> cannot appear as a child of <option>.\n' +
48
+ ' in div (at **)\n' +
49
' in option (at **)',
50
);
48
- expect(node.innerHTML).toBe('1 [object Object] 2');
51
+ expect(node.innerHTML).toBe('1 <div></div> 2');
52
+ ReactTestUtils.renderIntoDocument(el);
53
+ });
54
+
55
+ it('should warn for component child if no value prop is provided', () => {
56
+ function Foo() {
57
+ return '2';
58
+ }
59
+ const el = (
60
+ <option>
61
+ {1} <Foo /> {3}
62
+ </option>
63
+ );
64
+ let node;
65
+ expect(() => {
66
+ node = ReactTestUtils.renderIntoDocument(el);
67
+ }).toErrorDev(
68
+ 'Cannot infer the option value of complex children. ' +
69
+ 'Pass a `value` prop or use a plain string as children to <option>.',
70
+ );
71
+ expect(node.innerHTML).toBe('1 2 3');
72
+ ReactTestUtils.renderIntoDocument(el);
73
+ });
74
+
75
+ it('should not warn for component child if value prop is provided', () => {
76
+ function Foo() {
77
+ return '2';
78
+ }
79
+ const el = (
80
+ <option value="123">
81
+ {1} <Foo /> {3}
82
+ </option>
83
+ );
84
+ const node = ReactTestUtils.renderIntoDocument(el);
85
+ expect(node.innerHTML).toBe('1 2 3');
86
ReactTestUtils.renderIntoDocument(el);
87
});
88
@@ -91,7 +128,7 @@ describe('ReactDOMOption', () => {
128
129
it('should support element-ish child', () => {
130
// This is similar to <fbt>.
94
- // It's important that we toString it.
131
+ // We don't toString it because you must instead provide a value prop.
132
const obj = {
133
$$typeof: Symbol.for('react.element'),
134
type: props => props.content,
@@ -105,37 +142,42 @@ describe('ReactDOMOption', () => {
142
},
143
};
144
108
- let node = ReactTestUtils.renderIntoDocument(<option>{obj}</option>);
145
+ let node = ReactTestUtils.renderIntoDocument(
146
+ <option value="a">{obj}</option>,
147
+ );
148
expect(node.innerHTML).toBe('hello');
149
111
- node = ReactTestUtils.renderIntoDocument(<option>{[obj]}</option>);
150
+ node = ReactTestUtils.renderIntoDocument(
151
+ <option value="b">{[obj]}</option>,
152
+ );
153
expect(node.innerHTML).toBe('hello');
154
114
- expect(() => {
115
- node = ReactTestUtils.renderIntoDocument(
116
- <option>
117
- {obj}
118
- <span />
119
- </option>,
120
- );
121
- }).toErrorDev(
122
- 'Only strings and numbers are supported as <option> children.',
155
+ node = ReactTestUtils.renderIntoDocument(
156
+ <option value={obj}>{obj}</option>,
157
);
124
- expect(node.innerHTML).toBe('hello[object Object]');
158
+ expect(node.innerHTML).toBe('hello');
159
+ expect(node.value).toBe('hello');
160
161
node = ReactTestUtils.renderIntoDocument(
127
- <option>
162
+ <option value={obj}>
163
{'1'}
164
{obj}
165
{2}
166
</option>,
167
);
168
expect(node.innerHTML).toBe('1hello2');
169
+ expect(node.value).toBe('hello');
170
});
171
172
it('should be able to use dangerouslySetInnerHTML on option', () => {
173
const stub = <option dangerouslySetInnerHTML={{__html: 'foobar'}} />;
138
- const node = ReactTestUtils.renderIntoDocument(stub);
174
+ let node;
175
+ expect(() => {
176
+ node = ReactTestUtils.renderIntoDocument(stub);
177
+ }).toErrorDev(
178
+ 'Pass a `value` prop if you set dangerouslyInnerHTML so React knows which value should be selected.\n' +
179
+ ' in option (at **)',
180
+ );
181
182
expect(node.innerHTML).toBe('foobar');
183
});
@@ -169,4 +211,39 @@ describe('ReactDOMOption', () => {
211
ReactDOM.render(<select value="gorilla">{options}</select>, container);
212
expect(node.selectedIndex).toEqual(2);
213
});
214
+
215
+ it('generates a warning and hydration error when an invalid nested tag is used as a child', () => {
216
+ const ref = React.createRef();
217
+ const children = (
218
+ <select readOnly={true} value="bar">
219
+ <option value="bar">
220
+ {['Bar', false, 'Foo', <div key="1" ref={ref} />, 'Baz']}
221
+ </option>
222
+ </select>
223
+ );
224
+
225
+ const container = document.createElement('div');
226
+
227
+ container.innerHTML = ReactDOMServer.renderToString(children);
228
+
229
+ expect(container.firstChild.getAttribute('value')).toBe(null);
230
+ expect(container.firstChild.getAttribute('defaultValue')).toBe(null);
231
+
232
+ const option = container.firstChild.firstChild;
233
+ expect(option.nodeName).toBe('OPTION');
234
+
235
+ expect(option.textContent).toBe('BarFooBaz');
236
+ expect(option.selected).toBe(true);
237
+
238
+ expect(() => ReactDOM.hydrate(children, container)).toErrorDev([
239
+ 'Text content did not match. Server: "FooBaz" Client: "Foo"',
240
+ 'validateDOMNesting(...): <div> cannot appear as a child of <option>.',
241
+ ]);
242
+
243
+ expect(option.textContent).toBe('BarFooBaz');
244
+ expect(option.selected).toBe(true);
245
+
246
+ expect(ref.current.nodeName).toBe('DIV');
247
+ expect(ref.current.parentNode).toBe(option);
248
+ });
249
});
packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js
+12
-16
@@ -156,13 +156,12 @@ describe('ReactDOMServerIntegrationSelect', () => {
156
</option>
157
<option
158
id="baz"
159
- value="baz"
159
dangerouslySetInnerHTML={{
161
- __html: 'Baz',
160
+ __html: 'Baz', // This warns because no value prop is passed.
161
}}
162
/>
163
</select>,
165
- 1,
164
+ 2,
165
);
166
expectSelectValue(e, 'bar');
167
},
@@ -228,26 +227,23 @@ describe('ReactDOMServerIntegrationSelect', () => {
227
</select>,
228
);
229
const option = e.options[0];
231
- expect(option.childNodes.length).toBe(1);
232
- expect(option.childNodes[0].nodeType).toBe(3);
233
- expect(option.childNodes[0].nodeValue).toBe('A B');
230
+ expect(option.textContent).toBe('A B');
231
+ expect(option.value).toBe('bar');
232
+ expect(option.selected).toBe(true);
233
});
234
235
itRenders(
237
- 'a select option with flattened children and a warning',
236
+ 'a select option with flattened children no value',
237
async render => {
238
const e = await render(
240
- <select readOnly={true} value="bar">
241
- <option value="bar">
242
- {['Bar', false, 'Foo', <div key="1" />, 'Baz']}
243
- </option>
239
+ <select value="A B" readOnly={true}>
240
+ <option>A {'B'}</option>
241
</select>,
245
- 1,
242
);
247
- expect(e.getAttribute('value')).toBe(null);
248
- expect(e.getAttribute('defaultValue')).toBe(null);
249
- expect(e.firstChild.innerHTML).toBe('BarFoo[object Object]Baz');
250
- expect(e.firstChild.selected).toBe(true);
243
+ const option = e.options[0];
244
+ expect(option.textContent).toBe('A B');
245
+ expect(option.value).toBe('A B');
246
+ expect(option.selected).toBe(true);
247
},
248
);
249
});
packages/react-dom/src/client/ReactDOMComponent.js
+1
-7
@@ -29,7 +29,6 @@ import {
29
restoreControlledState as ReactDOMInputRestoreControlledState,
30
} from './ReactDOMInput';
31
import {
32
- getHostProps as ReactDOMOptionGetHostProps,
32
postMountWrapper as ReactDOMOptionPostMountWrapper,
33
validateProps as ReactDOMOptionValidateProps,
34
} from './ReactDOMOption';
@@ -535,7 +534,7 @@ export function setInitialProperties(
534
break;
535
case 'option':
536
ReactDOMOptionValidateProps(domElement, rawProps);
538
- props = ReactDOMOptionGetHostProps(domElement, rawProps);
537
+ props = rawProps;
538
break;
539
case 'select':
540
ReactDOMSelectInitWrapperState(domElement, rawProps);
@@ -615,11 +614,6 @@ export function diffProperties(
614
nextProps = ReactDOMInputGetHostProps(domElement, nextRawProps);
615
updatePayload = [];
616
break;
618
- case 'option':
619
- lastProps = ReactDOMOptionGetHostProps(domElement, lastRawProps);
620
- nextProps = ReactDOMOptionGetHostProps(domElement, nextRawProps);
621
- updatePayload = [];
622
- break;
617
case 'select':
618
lastProps = ReactDOMSelectGetHostProps(domElement, lastRawProps);
619
nextProps = ReactDOMSelectGetHostProps(domElement, nextRawProps);
packages/react-dom/src/client/ReactDOMHostConfig.js
-1
@@ -351,7 +351,6 @@ export function prepareUpdate(
351
export function shouldSetTextContent(type: string, props: Props): boolean {
352
return (
353
type === 'textarea' ||
354
- type === 'option' ||
354
type === 'noscript' ||
355
typeof props.children === 'string' ||
356
typeof props.children === 'number' ||
packages/react-dom/src/client/ReactDOMOption.js
+25
-51
@@ -12,27 +12,7 @@ import {getToStringValue, toString} from './ToStringValue';
12
13
let didWarnSelectedSetOnOption = false;
14
let didWarnInvalidChild = false;
15
-
16
-function flattenChildren(children) {
17
- let content = '';
18
-
19
- // Flatten children. We'll warn if they are invalid
20
- // during validateProps() which runs for hydration too.
21
- // Note that this would throw on non-element objects.
22
- // Elements are stringified (which is normally irrelevant
23
- // but matters for <fbt>).
24
- Children.forEach(children, function(child) {
25
- if (child == null) {
26
- return;
27
- }
28
- content += (child: any);
29
- // Note: we don't warn about invalid children here.
30
- // Instead, this is done separately below so that
31
- // it happens during the hydration code path too.
32
- });
33
-
34
- return content;
35
-}
15
+let didWarnInvalidInnerHTML = false;
16
17
/**
18
* Implements an <option> host component that warns when `selected` is set.
@@ -40,28 +20,33 @@ function flattenChildren(children) {
20
21
export function validateProps(element: Element, props: Object) {
22
if (__DEV__) {
43
- // This mirrors the code path above, but runs for hydration too.
44
- // Warn about invalid children here so that client and hydration are consistent.
45
- // TODO: this seems like it could cause a DEV-only throw for hydration
46
- // if children contains a non-element object. We should try to avoid that.
47
- if (typeof props.children === 'object' && props.children !== null) {
48
- Children.forEach(props.children, function(child) {
49
- if (child == null) {
50
- return;
51
- }
52
- if (typeof child === 'string' || typeof child === 'number') {
53
- return;
54
- }
55
- if (typeof (child: any).type !== 'string') {
56
- return;
57
- }
58
- if (!didWarnInvalidChild) {
59
- didWarnInvalidChild = true;
23
+ // If a value is not provided, then the children must be simple.
24
+ if (props.value == null) {
25
+ if (typeof props.children === 'object' && props.children !== null) {
26
+ Children.forEach(props.children, function(child) {
27
+ if (child == null) {
28
+ return;
29
+ }
30
+ if (typeof child === 'string' || typeof child === 'number') {
31
+ return;
32
+ }
33
+ if (!didWarnInvalidChild) {
34
+ didWarnInvalidChild = true;
35
+ console.error(
36
+ 'Cannot infer the option value of complex children. ' +
37
+ 'Pass a `value` prop or use a plain string as children to <option>.',
38
+ );
39
+ }
40
+ });
41
+ } else if (props.dangerouslySetInnerHTML != null) {
42
+ if (!didWarnInvalidInnerHTML) {
43
+ didWarnInvalidInnerHTML = true;
44
console.error(
61
- 'Only strings and numbers are supported as <option> children.',
45
+ 'Pass a `value` prop if you set dangerouslyInnerHTML so React knows ' +
46
+ 'which value should be selected.',
47
);
48
}
64
- });
49
+ }
50
}
51
52
// TODO: Remove support for `selected` in <option>.
@@ -81,14 +66,3 @@ export function postMountWrapper(element: Element, props: Object) {
66
element.setAttribute('value', toString(getToStringValue(props.value)));
67
}
68
}
84
-
85
-export function getHostProps(element: Element, props: Object) {
86
- const hostProps = {children: undefined, ...props};
87
- const content = flattenChildren(props.children);
88
-
89
- if (content) {
90
- hostProps.children = content;
91
- }
92
-
93
- return hostProps;
94
-}
packages/react-dom/src/server/ReactDOMServerFormatConfig.js
+15
-2
@@ -563,6 +563,7 @@ let didWarnDefaultChecked = false;
563
let didWarnDefaultSelectValue = false;
564
let didWarnDefaultTextareaValue = false;
565
let didWarnInvalidOptionChildren = false;
566
+let didWarnInvalidOptionInnerHTML = false;
567
let didWarnSelectedSetOnOption = false;
568
569
function checkSelectProp(props, propName) {
@@ -667,7 +668,8 @@ function flattenOptionChildren(children: mixed): string {
668
) {
669
didWarnInvalidOptionChildren = true;
670
console.error(
670
- 'Only strings and numbers are supported as <option> children.',
671
+ 'Cannot infer the option value of complex children. ' +
672
+ 'Pass a `value` prop or use a plain string as children to <option>.',
673
);
674
}
675
}
@@ -736,7 +738,18 @@ function pushStartOption(
738
if (value !== null) {
739
stringValue = '' + value;
740
} else {
739
- stringValue = children = flattenOptionChildren(children);
741
+ if (__DEV__) {
742
+ if (innerHTML !== null) {
743
+ if (!didWarnInvalidOptionInnerHTML) {
744
+ didWarnInvalidOptionInnerHTML = true;
745
+ console.error(
746
+ 'Pass a `value` prop if you set dangerouslyInnerHTML so React knows ' +
747
+ 'which value should be selected.',
748
+ );
749
+ }
750
+ }
751
+ }
752
+ stringValue = flattenOptionChildren(children);
753
}
754
if (isArray(selectedValue)) {
755
// multiple
packages/react-dom/src/server/ReactPartialRenderer.js
+15
-5
@@ -188,6 +188,7 @@ let didWarnDefaultChecked = false;
188
let didWarnDefaultSelectValue = false;
189
let didWarnDefaultTextareaValue = false;
190
let didWarnInvalidOptionChildren = false;
191
+let didWarnInvalidOptionInnerHTML = false;
192
const didWarnAboutNoopUpdateForComponent = {};
193
const didWarnAboutBadClass = {};
194
const didWarnAboutModulePatternComponent = {};
@@ -334,7 +335,8 @@ function flattenOptionChildren(children: mixed): ?string {
335
) {
336
didWarnInvalidOptionChildren = true;
337
console.error(
337
- 'Only strings and numbers are supported as <option> children.',
338
+ 'Cannot infer the option value of complex children. ' +
339
+ 'Pass a `value` prop or use a plain string as children to <option>.',
340
);
341
}
342
}
@@ -1507,13 +1509,23 @@ class ReactDOMServerRenderer {
1509
} else if (tag === 'option') {
1510
let selected = null;
1511
const selectValue = this.currentSelectValue;
1510
- const optionChildren = flattenOptionChildren(props.children);
1512
if (selectValue != null) {
1513
let value;
1514
if (props.value != null) {
1515
value = props.value + '';
1516
} else {
1516
- value = optionChildren;
1517
+ if (__DEV__) {
1518
+ if (props.dangerouslySetInnerHTML != null) {
1519
+ if (!didWarnInvalidOptionInnerHTML) {
1520
+ didWarnInvalidOptionInnerHTML = true;
1521
+ console.error(
1522
+ 'Pass a `value` prop if you set dangerouslyInnerHTML so React knows ' +
1523
+ 'which value should be selected.',
1524
+ );
1525
+ }
1526
+ }
1527
+ }
1528
+ value = flattenOptionChildren(props.children);
1529
}
1530
selected = false;
1531
if (isArray(selectValue)) {
@@ -1531,12 +1543,10 @@ class ReactDOMServerRenderer {
1543
props = Object.assign(
1544
{
1545
selected: undefined,
1534
- children: undefined,
1546
},
1547
props,
1548
{
1549
selected: selected,
1539
- children: optionChildren,
1550
},
1551
);
1552
}