Stringify <option> children (#13465)

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

@@ -15,6 +15,7 @@ describe('ReactDOMOption', () => {
  let ReactTestUtils;
 
  beforeEach(() => {
+ jest.resetModules();
  React = require('react');
  ReactDOM = require('react-dom');
  ReactTestUtils = require('react-dom/test-utils');
@@ -41,9 +42,10 @@ describe('ReactDOMOption', () => {
  expect(() => {
  node = ReactTestUtils.renderIntoDocument(el);
  }).toWarnDev(
- '<div> cannot appear as a child of <option>.\n' + ' in option (at **)',
+ 'Only strings and numbers are supported as <option> children.\n' +
+ ' in option (at **)',
  );
- expect(node.innerHTML).toBe('1 2');
+ expect(node.innerHTML).toBe('1 [object Object] 2');
  ReactTestUtils.renderIntoDocument(el);
  });
 
@@ -61,6 +63,76 @@ describe('ReactDOMOption', () => {
  expect(node.innerHTML).toBe('1 2');
  });
 
+ it('should throw on object children', () => {
+ expect(() => {
+ ReactTestUtils.renderIntoDocument(<option>{{}}</option>);
+ }).toThrow('Objects are not valid as a React child');
+ expect(() => {
+ ReactTestUtils.renderIntoDocument(<option>{[{}]}</option>);
+ }).toThrow('Objects are not valid as a React child');
+ expect(() => {
+ ReactTestUtils.renderIntoDocument(
+ <option>
+ {{}}
+ <span />
+ </option>,
+ );
+ }).toThrow('Objects are not valid as a React child');
+ expect(() => {
+ ReactTestUtils.renderIntoDocument(
+ <option>
+ {'1'}
+ {{}}
+ {2}
+ </option>,
+ );
+ }).toThrow('Objects are not valid as a React child');
+ });
+
+ it('should support element-ish child', () => {
+ // This is similar to <fbt>.
+ // It's important that we toString it.
+ let obj = {
+ $$typeof: Symbol.for('react.element'),
+ type: props => props.content,
+ ref: null,
+ key: null,
+ props: {
+ content: 'hello',
+ },
+ toString() {
+ return this.props.content;
+ },
+ };
+
+ let node = ReactTestUtils.renderIntoDocument(<option>{obj}</option>);
+ expect(node.innerHTML).toBe('hello');
+
+ node = ReactTestUtils.renderIntoDocument(<option>{[obj]}</option>);
+ expect(node.innerHTML).toBe('hello');
+
+ expect(() => {
+ node = ReactTestUtils.renderIntoDocument(
+ <option>
+ {obj}
+ <span />
+ </option>,
+ );
+ }).toWarnDev(
+ 'Only strings and numbers are supported as <option> children.',
+ );
+ expect(node.innerHTML).toBe('hello[object Object]');
+
+ node = ReactTestUtils.renderIntoDocument(
+ <option>
+ {'1'}
+ {obj}
+ {2}
+ </option>,
+ );
+ expect(node.innerHTML).toBe('1hello2');
+ });
+
  it('should be able to use dangerouslySetInnerHTML on option', () => {
  let stub = <option dangerouslySetInnerHTML={{__html: 'foobar'}} />;
  const node = ReactTestUtils.renderIntoDocument(stub);

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

@@ -439,7 +439,7 @@ describe('ReactDOMServerIntegration', () => {
  );
  expect(e.getAttribute('value')).toBe(null);
  expect(e.getAttribute('defaultValue')).toBe(null);
- expect(e.firstChild.innerHTML).toBe('BarFooBaz');
+ expect(e.firstChild.innerHTML).toBe('BarFoo[object Object]Baz');
  expect(e.firstChild.selected).toBe(true);
  },
  );

packages/react-dom/src/client/ReactDOMFiberOption.js

@@ -9,23 +9,24 @@
 
 import React from 'react';
 import warning from 'shared/warning';
-import {validateDOMNesting, updatedAncestorInfo} from './validateDOMNesting';
 import {getToStringValue, toString} from './ToStringValue';
 
 let didWarnSelectedSetOnOption = false;
+let didWarnInvalidChild = false;
 
 function flattenChildren(children) {
  let content = '';
 
- // Flatten children and warn if they aren't strings or numbers;
- // invalid types are ignored.
+ // Flatten children. We'll warn if they are invalid
+ // during validateProps() which runs for hydration too.
+ // Note that this would throw on non-element objects.
+ // Elements are stringified (which is normally irrelevant
+ // but matters for <fbt>).
  React.Children.forEach(children, function(child) {
  if (child == null) {
  return;
  }
- if (typeof child === 'string' || typeof child === 'number') {
- content += child;
- }
+ content += child;
  // Note: we don't warn about invalid children here.
  // Instead, this is done separately below so that
  // it happens during the hydration codepath too.
@@ -40,7 +41,10 @@ function flattenChildren(children) {
 
 export function validateProps(element: Element, props: Object) {
  if (__DEV__) {
- // Warn about invalid children, mirroring the logic above.
+ // This mirrors the codepath above, but runs for hydration too.
+ // Warn about invalid children here so that client and hydration are consistent.
+ // TODO: this seems like it could cause a DEV-only throw for hydration
+ // if children contains a non-element object. We should try to avoid that.
  if (typeof props.children === 'object' && props.children !== null) {
  React.Children.forEach(props.children, function(child) {
  if (child == null) {
@@ -49,13 +53,16 @@ export function validateProps(element: Element, props: Object) {
  if (typeof child === 'string' || typeof child === 'number') {
  return;
  }
- // This is not real ancestor info but it's close enough
- // to produce a useful warning for invalid children.
- // We don't have access to the real one because the <option>
- // fiber has already been popped, and threading it through
- // is needlessly annoying.
- const ancestorInfo = updatedAncestorInfo(null, 'option');
- validateDOMNesting(child.type, null, ancestorInfo);
+ if (typeof child.type !== 'string') {
+ return;
+ }
+ if (!didWarnInvalidChild) {
+ didWarnInvalidChild = true;
+ warning(
+ false,
+ 'Only strings and numbers are supported as <option> children.',
+ );
+ }
  });
  }
 

packages/react-dom/src/server/ReactPartialRenderer.js

@@ -293,17 +293,18 @@ function flattenOptionChildren(children: mixed): ?string {
  if (child == null) {
  return;
  }
- if (typeof child === 'string' || typeof child === 'number') {
- content += child;
- } else {
- if (__DEV__) {
- if (!didWarnInvalidOptionChildren) {
- didWarnInvalidOptionChildren = true;
- warningWithoutStack(
- false,
- 'Only strings and numbers are supported as <option> children.',
- );
- }
+ content += child;
+ if (__DEV__) {
+ if (
+ !didWarnInvalidOptionChildren &&
+ typeof child !== 'string' &&
+ typeof child !== 'number'
+ ) {
+ didWarnInvalidOptionChildren = true;
+ warning(
+ false,
+ 'Only strings and numbers are supported as <option> children.',
+ );
  }
  }
  });