From 0182a74632b66a226921ca13dcfe359c67e15c6f Mon Sep 17 00:00:00 2001
From: Konstantin Yakushin <y231966mdk@gmail.com>
Date: Wed, 1 Aug 2018 18:16:34 +0300
Subject: [PATCH] Fix a crash when using dynamic children in <option> tag
 (#13261)

* Make option children a text content by default

fix #11911

* Apply requested changes

- Remove meaningless comments
- revert scripts/rollup/results.json

* remove empty row

* Update comment

* Add a simple unit-test

* [WIP: no flow] Pass through hostContext

* [WIP: no flow] Give better description for test

* Fixes

* Don't pass hostContext through

It ended up being more complicated than I thought.

* Also warn on hydration
---
 .../src/components/fixtures/selects/index.js  | 25 +++++++++++++++
 .../src/__tests__/ReactDOMOption-test.js      |  4 +--
 .../src/__tests__/ReactDOMSelect-test.js      | 32 +++++++++++++++++++
 .../ReactDOMServerIntegrationForms-test.js    | 15 +++++++++
 .../ReactDOMServerIntegrationTestUtils.js     |  2 +-
 .../src/client/ReactDOMFiberOption.js         | 30 +++++++++++++++--
 .../src/client/ReactDOMHostConfig.js          |  1 +
 7 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/fixtures/dom/src/components/fixtures/selects/index.js b/fixtures/dom/src/components/fixtures/selects/index.js
index 5460919cc2..31d7f4436d 100644
--- a/fixtures/dom/src/components/fixtures/selects/index.js
+++ b/fixtures/dom/src/components/fixtures/selects/index.js
@@ -158,6 +158,31 @@ class SelectFixture extends React.Component {
             </form>
           </div>
         </TestCase>
+
+        <TestCase
+          title="An option which contains conditional render fails"
+          relatedIssues="11911">
+          <TestCase.Steps>
+            <li>Select any option</li>
+          </TestCase.Steps>
+          <TestCase.ExpectedResult>
+            Option should be set
+          </TestCase.ExpectedResult>
+
+          <div className="test-fixture">
+            <select value={this.state.value} onChange={this.onChange}>
+              <option value="red">
+                red {this.state.value === 'red' && 'is chosen '} TextNode
+              </option>
+              <option value="blue">
+                blue {this.state.value === 'blue' && 'is chosen '} TextNode
+              </option>
+              <option value="green">
+                green {this.state.value === 'green' && 'is chosen '} TextNode
+              </option>
+            </select>
+          </div>
+        </TestCase>
       </FixtureSet>
     );
   }
diff --git a/packages/react-dom/src/__tests__/ReactDOMOption-test.js b/packages/react-dom/src/__tests__/ReactDOMOption-test.js
index 8cbd0118d2..bd30ae23a1 100644
--- a/packages/react-dom/src/__tests__/ReactDOMOption-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMOption-test.js
@@ -41,9 +41,7 @@ describe('ReactDOMOption', () => {
     expect(() => {
       node = ReactTestUtils.renderIntoDocument(el);
     }).toWarnDev(
-      '<div> cannot appear as a child of <option>.\n' +
-        '    in div (at **)\n' +
-        '    in option (at **)',
+      '<div> cannot appear as a child of <option>.\n' + '    in option (at **)',
     );
     expect(node.innerHTML).toBe('1  2');
     ReactTestUtils.renderIntoDocument(el);
diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js
index e94cc7895c..04efbec66a 100644
--- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js
@@ -545,6 +545,38 @@ describe('ReactDOMSelect', () => {
     expect(node.options[2].selected).toBe(false); // gorilla
   });
 
+  it('should support options with dynamic children', () => {
+    const container = document.createElement('div');
+
+    let node;
+
+    function App({value}) {
+      return (
+        <select value={value} ref={n => (node = n)} onChange={noop}>
+          <option key="monkey" value="monkey">
+            A monkey {value === 'monkey' ? 'is chosen' : null}!
+          </option>
+          <option key="giraffe" value="giraffe">
+            A giraffe {value === 'giraffe' && 'is chosen'}!
+          </option>
+          <option key="gorilla" value="gorilla">
+            A gorilla {value === 'gorilla' && 'is chosen'}!
+          </option>
+        </select>
+      );
+    }
+
+    ReactDOM.render(<App value="monkey" />, container);
+    expect(node.options[0].selected).toBe(true); // monkey
+    expect(node.options[1].selected).toBe(false); // giraffe
+    expect(node.options[2].selected).toBe(false); // gorilla
+
+    ReactDOM.render(<App value="giraffe" />, container);
+    expect(node.options[0].selected).toBe(false); // monkey
+    expect(node.options[1].selected).toBe(true); // giraffe
+    expect(node.options[2].selected).toBe(false); // gorilla
+  });
+
   it('should warn if value is null', () => {
     expect(() =>
       ReactTestUtils.renderIntoDocument(
diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js
index bc1db8d569..0f6a0cab49 100644
--- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js
@@ -413,6 +413,21 @@ describe('ReactDOMServerIntegration', () => {
           expectSelectValue(e, 'bar');
         },
       );
+
+      itRenders('an option with flattened children', async render => {
+        const e = await render(
+          <select readOnly={true} value="bar">
+            <option value="bar">
+              {['Bar', false, 'Foo', <div key="1" />, 'Baz']}
+            </option>
+          </select>,
+          1,
+        );
+        expect(e.getAttribute('value')).toBe(null);
+        expect(e.getAttribute('defaultValue')).toBe(null);
+        expect(e.firstChild.innerHTML).toBe('BarFooBaz');
+        expect(e.firstChild.selected).toBe(true);
+      });
     });
 
     describe('user interaction', function() {
diff --git a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js
index a83e1ec181..daf7b49d7f 100644
--- a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js
+++ b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js
@@ -58,7 +58,7 @@ module.exports = function(initModules) {
       if (console.error.calls.count() > 0) {
         console.log(`We saw these warnings:`);
         for (let i = 0; i < console.error.calls.count(); i++) {
-          console.log(console.error.calls.argsFor(i)[0]);
+          console.log(...console.error.calls.argsFor(i));
         }
       }
     }
diff --git a/packages/react-dom/src/client/ReactDOMFiberOption.js b/packages/react-dom/src/client/ReactDOMFiberOption.js
index e5c04d2392..1de8f8c693 100644
--- a/packages/react-dom/src/client/ReactDOMFiberOption.js
+++ b/packages/react-dom/src/client/ReactDOMFiberOption.js
@@ -9,6 +9,7 @@
 
 import React from 'react';
 import warning from 'shared/warning';
+import validateDOMNesting from './validateDOMNesting';
 
 let didWarnSelectedSetOnOption = false;
 
@@ -17,8 +18,6 @@ function flattenChildren(children) {
 
   // Flatten children and warn if they aren't strings or numbers;
   // invalid types are ignored.
-  // We can silently skip them because invalid DOM nesting warning
-  // catches these cases in Fiber.
   React.Children.forEach(children, function(child) {
     if (child == null) {
       return;
@@ -26,6 +25,9 @@ function flattenChildren(children) {
     if (typeof child === 'string' || typeof child === 'number') {
       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.
   });
 
   return content;
@@ -36,8 +38,30 @@ function flattenChildren(children) {
  */
 
 export function validateProps(element: Element, props: Object) {
-  // TODO (yungsters): Remove support for `selected` in <option>.
   if (__DEV__) {
+    // Warn about invalid children, mirroring the logic above.
+    if (typeof props.children === 'object' && props.children !== null) {
+      React.Children.forEach(props.children, function(child) {
+        if (child == null) {
+          return;
+        }
+        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 = validateDOMNesting.updatedAncestorInfo(
+          null,
+          'option',
+        );
+        validateDOMNesting(child.type, null, ancestorInfo);
+      });
+    }
+
+    // TODO: Remove support for `selected` in <option>.
     if (props.selected != null && !didWarnSelectedSetOnOption) {
       warning(
         false,
diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js
index 412f06a9aa..e8f6a5bbde 100644
--- a/packages/react-dom/src/client/ReactDOMHostConfig.js
+++ b/packages/react-dom/src/client/ReactDOMHostConfig.js
@@ -248,6 +248,7 @@ export function prepareUpdate(
 export function shouldSetTextContent(type: string, props: Props): boolean {
   return (
     type === 'textarea' ||
+    type === 'option' ||
     typeof props.children === 'string' ||
     typeof props.children === 'number' ||
     (typeof props.dangerouslySetInnerHTML === 'object' &&