Stringify <option> children (#13465)
Dan Abramov committed
Aug 23, 2018 at 00:24 UTC
5cefd9b1e230fbf680ffbe86362387749f584386
4 files changed
+108
-28
packages/react-dom/src/__tests__/ReactDOMOption-test.js
+74
-2
@@ -15,6 +15,7 @@ describe('ReactDOMOption', () => {
15
let ReactTestUtils;
16
17
beforeEach(() => {
18
+ jest.resetModules();
19
React = require('react');
20
ReactDOM = require('react-dom');
21
ReactTestUtils = require('react-dom/test-utils');
@@ -41,9 +42,10 @@ describe('ReactDOMOption', () => {
42
expect(() => {
43
node = ReactTestUtils.renderIntoDocument(el);
44
}).toWarnDev(
44
- '<div> cannot appear as a child of <option>.\n' + ' in option (at **)',
45
+ 'Only strings and numbers are supported as <option> children.\n' +
46
+ ' in option (at **)',
47
);
46
- expect(node.innerHTML).toBe('1 2');
48
+ expect(node.innerHTML).toBe('1 [object Object] 2');
49
ReactTestUtils.renderIntoDocument(el);
50
});
51
@@ -61,6 +63,76 @@ describe('ReactDOMOption', () => {
63
expect(node.innerHTML).toBe('1 2');
64
});
65
66
+ it('should throw on object children', () => {
67
+ expect(() => {
68
+ ReactTestUtils.renderIntoDocument(<option>{{}}</option>);
69
+ }).toThrow('Objects are not valid as a React child');
70
+ expect(() => {
71
+ ReactTestUtils.renderIntoDocument(<option>{[{}]}</option>);
72
+ }).toThrow('Objects are not valid as a React child');
73
+ expect(() => {
74
+ ReactTestUtils.renderIntoDocument(
75
+ <option>
76
+ {{}}
77
+ <span />
78
+ </option>,
79
+ );
80
+ }).toThrow('Objects are not valid as a React child');
81
+ expect(() => {
82
+ ReactTestUtils.renderIntoDocument(
83
+ <option>
84
+ {'1'}
85
+ {{}}
86
+ {2}
87
+ </option>,
88
+ );
89
+ }).toThrow('Objects are not valid as a React child');
90
+ });
91
+
92
+ it('should support element-ish child', () => {
93
+ // This is similar to <fbt>.
94
+ // It's important that we toString it.
95
+ let obj = {
96
+ $$typeof: Symbol.for('react.element'),
97
+ type: props => props.content,
98
+ ref: null,
99
+ key: null,
100
+ props: {
101
+ content: 'hello',
102
+ },
103
+ toString() {
104
+ return this.props.content;
105
+ },
106
+ };
107
+
108
+ let node = ReactTestUtils.renderIntoDocument(<option>{obj}</option>);
109
+ expect(node.innerHTML).toBe('hello');
110
+
111
+ node = ReactTestUtils.renderIntoDocument(<option>{[obj]}</option>);
112
+ expect(node.innerHTML).toBe('hello');
113
+
114
+ expect(() => {
115
+ node = ReactTestUtils.renderIntoDocument(
116
+ <option>
117
+ {obj}
118
+ <span />
119
+ </option>,
120
+ );
121
+ }).toWarnDev(
122
+ 'Only strings and numbers are supported as <option> children.',
123
+ );
124
+ expect(node.innerHTML).toBe('hello[object Object]');
125
+
126
+ node = ReactTestUtils.renderIntoDocument(
127
+ <option>
128
+ {'1'}
129
+ {obj}
130
+ {2}
131
+ </option>,
132
+ );
133
+ expect(node.innerHTML).toBe('1hello2');
134
+ });
135
+
136
it('should be able to use dangerouslySetInnerHTML on option', () => {
137
let stub = <option dangerouslySetInnerHTML={{__html: 'foobar'}} />;
138
const node = ReactTestUtils.renderIntoDocument(stub);
packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js
+1
-1
@@ -439,7 +439,7 @@ describe('ReactDOMServerIntegration', () => {
439
);
440
expect(e.getAttribute('value')).toBe(null);
441
expect(e.getAttribute('defaultValue')).toBe(null);
442
- expect(e.firstChild.innerHTML).toBe('BarFooBaz');
442
+ expect(e.firstChild.innerHTML).toBe('BarFoo[object Object]Baz');
443
expect(e.firstChild.selected).toBe(true);
444
},
445
);
packages/react-dom/src/client/ReactDOMFiberOption.js
+21
-14
@@ -9,23 +9,24 @@
9
10
import React from 'react';
11
import warning from 'shared/warning';
12
-import {validateDOMNesting, updatedAncestorInfo} from './validateDOMNesting';
12
import {getToStringValue, toString} from './ToStringValue';
13
14
let didWarnSelectedSetOnOption = false;
15
+let didWarnInvalidChild = false;
16
17
function flattenChildren(children) {
18
let content = '';
19
20
- // Flatten children and warn if they aren't strings or numbers;
21
- // invalid types are ignored.
20
+ // Flatten children. We'll warn if they are invalid
21
+ // during validateProps() which runs for hydration too.
22
+ // Note that this would throw on non-element objects.
23
+ // Elements are stringified (which is normally irrelevant
24
+ // but matters for <fbt>).
25
React.Children.forEach(children, function(child) {
26
if (child == null) {
27
return;
28
}
26
- if (typeof child === 'string' || typeof child === 'number') {
27
- content += child;
28
- }
29
+ content += child;
30
// Note: we don't warn about invalid children here.
31
// Instead, this is done separately below so that
32
// it happens during the hydration codepath too.
@@ -40,7 +41,10 @@ function flattenChildren(children) {
41
42
export function validateProps(element: Element, props: Object) {
43
if (__DEV__) {
43
- // Warn about invalid children, mirroring the logic above.
44
+ // This mirrors the codepath above, but runs for hydration too.
45
+ // Warn about invalid children here so that client and hydration are consistent.
46
+ // TODO: this seems like it could cause a DEV-only throw for hydration
47
+ // if children contains a non-element object. We should try to avoid that.
48
if (typeof props.children === 'object' && props.children !== null) {
49
React.Children.forEach(props.children, function(child) {
50
if (child == null) {
@@ -49,13 +53,16 @@ export function validateProps(element: Element, props: Object) {
53
if (typeof child === 'string' || typeof child === 'number') {
54
return;
55
}
52
- // This is not real ancestor info but it's close enough
53
- // to produce a useful warning for invalid children.
54
- // We don't have access to the real one because the <option>
55
- // fiber has already been popped, and threading it through
56
- // is needlessly annoying.
57
- const ancestorInfo = updatedAncestorInfo(null, 'option');
58
- validateDOMNesting(child.type, null, ancestorInfo);
56
+ if (typeof child.type !== 'string') {
57
+ return;
58
+ }
59
+ if (!didWarnInvalidChild) {
60
+ didWarnInvalidChild = true;
61
+ warning(
62
+ false,
63
+ 'Only strings and numbers are supported as <option> children.',
64
+ );
65
+ }
66
});
67
}
68
packages/react-dom/src/server/ReactPartialRenderer.js
+12
-11
@@ -293,17 +293,18 @@ function flattenOptionChildren(children: mixed): ?string {
293
if (child == null) {
294
return;
295
}
296
- if (typeof child === 'string' || typeof child === 'number') {
297
- content += child;
298
- } else {
299
- if (__DEV__) {
300
- if (!didWarnInvalidOptionChildren) {
301
- didWarnInvalidOptionChildren = true;
302
- warningWithoutStack(
303
- false,
304
- 'Only strings and numbers are supported as <option> children.',
305
- );
306
- }
296
+ content += child;
297
+ if (__DEV__) {
298
+ if (
299
+ !didWarnInvalidOptionChildren &&
300
+ typeof child !== 'string' &&
301
+ typeof child !== 'number'
302
+ ) {
303
+ didWarnInvalidOptionChildren = true;
304
+ warning(
305
+ false,
306
+ 'Only strings and numbers are supported as <option> children.',
307
+ );
308
}
309
}
310
});