From 5cefd9b1e230fbf680ffbe86362387749f584386 Mon Sep 17 00:00:00 2001
From: Dan Abramov <dan.abramov@gmail.com>
Date: Thu, 23 Aug 2018 00:24:05 +0100
Subject: [PATCH] Stringify <option> children (#13465)

---
 .../src/__tests__/ReactDOMOption-test.js      | 76 ++++++++++++++++++-
 .../ReactDOMServerIntegrationForms-test.js    |  2 +-
 .../src/client/ReactDOMFiberOption.js         | 35 +++++----
 .../src/server/ReactPartialRenderer.js        | 23 +++---
 4 files changed, 108 insertions(+), 28 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactDOMOption-test.js b/packages/react-dom/src/__tests__/ReactDOMOption-test.js
index bd30ae23a1dd5..09df9137ce927 100644
--- a/packages/react-dom/src/__tests__/ReactDOMOption-test.js
+++ b/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);
diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js
index 6e2d95fe50e54..83cb86de5e2fa 100644
--- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js
+++ b/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);
         },
       );
diff --git a/packages/react-dom/src/client/ReactDOMFiberOption.js b/packages/react-dom/src/client/ReactDOMFiberOption.js
index 9d905ceaaa018..117e679b531ac 100644
--- a/packages/react-dom/src/client/ReactDOMFiberOption.js
+++ b/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.',
+          );
+        }
       });
     }
 
diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js
index 02487cafa5398..230806536bf39 100644
--- a/packages/react-dom/src/server/ReactPartialRenderer.js
+++ b/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.',
+        );
       }
     }
   });