From e5d85fbe04dda86cd71e8a825537a70ec85a82df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Tue, 1 Nov 2016 18:54:20 -0700
Subject: [PATCH] Remove pendingUpdate optimization in ReactDOMSelect (#8175)

This removes an optimization that avoids call update on ReactDOMSelect
twice in an onChange event.

https://github.com/facebook/react/commit/2601b6a0b0c2b0bccaca67fbbfab6987ac5bef30#commitcomment-19643403

None of the other controlled components do this. The only reason to do it
here is because of the loop.

I'd like to remove this because I'd like to remove all the side-effects
that happen in onChange, other than user defined behavior. I'd also like to
get rid of state that track sequences. It is easier if everything is just
diffing.

Alternatively I can store the previous value that we processed and only
reprocess if the value has changed. However, that would requires the array
for multiple values to be immutable and I don't think we enforce that
right now.

In Fiber, I believe that we'll be able to batch both these updates into a
single commit.
---
 .../wrappers/__tests__/ReactDOMSelect-test.js |  4 +---
 .../stack/client/wrappers/ReactDOMSelect.js   | 24 +++++++------------
 2 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js
index 7849539903..c5933f738e 100644
--- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js
+++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js
@@ -497,9 +497,7 @@ describe('ReactDOMSelect', () => {
     stub = ReactDOM.render(stub, container);
     var node = ReactDOM.findDOMNode(stub);
 
-    expect(() => ReactTestUtils.Simulate.change(node)).not.toThrow(
-      "Cannot set property 'pendingUpdate' of null"
-    );
+    expect(() => ReactTestUtils.Simulate.change(node)).not.toThrow();
   });
 
   it('should select grandchild options nested inside an optgroup', () => {
diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js b/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js
index ff02764f2c..33b5ea4cf1 100644
--- a/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js
+++ b/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js
@@ -19,10 +19,8 @@ var warning = require('warning');
 
 var didWarnValueDefaultValue = false;
 
-function updateOptionsIfPendingUpdateAndMounted() {
-  if (this._rootNodeID && this._wrapperState.pendingUpdate) {
-    this._wrapperState.pendingUpdate = false;
-
+function forceUpdateIfMounted() {
+  if (this._rootNodeID) {
     var props = this._currentElement.props;
     var value = props.value;
 
@@ -89,15 +87,14 @@ function checkSelectPropTypes(inst, props) {
  * @private
  */
 function updateOptions(inst, multiple, propValue) {
-  var selectedValue, i;
   var options = ReactDOMComponentTree.getNodeFromInstance(inst).options;
 
   if (multiple) {
-    selectedValue = {};
-    for (i = 0; i < propValue.length; i++) {
+    let selectedValue = {};
+    for (let i = 0; i < propValue.length; i++) {
       selectedValue['' + propValue[i]] = true;
     }
-    for (i = 0; i < options.length; i++) {
+    for (let i = 0; i < options.length; i++) {
       var selected = selectedValue.hasOwnProperty(options[i].value);
       if (options[i].selected !== selected) {
         options[i].selected = selected;
@@ -106,8 +103,8 @@ function updateOptions(inst, multiple, propValue) {
   } else {
     // Do not set `select.value` as exact behavior isn't consistent across all
     // browsers for all cases.
-    selectedValue = '' + propValue;
-    for (i = 0; i < options.length; i++) {
+    let selectedValue = '' + propValue;
+    for (let i = 0; i < options.length; i++) {
       if (options[i].value === selectedValue) {
         options[i].selected = true;
         return;
@@ -149,7 +146,6 @@ var ReactDOMSelect = {
 
     var value = props.value;
     inst._wrapperState = {
-      pendingUpdate: false,
       initialValue: value != null ? value : props.defaultValue,
       listeners: null,
       onChange: _handleChange.bind(inst),
@@ -191,7 +187,6 @@ var ReactDOMSelect = {
 
     var value = props.value;
     if (value != null) {
-      inst._wrapperState.pendingUpdate = false;
       updateOptions(inst, Boolean(props.multiple), value);
     } else if (wasMultiple !== Boolean(props.multiple)) {
       // For simplicity, reapply `defaultValue` if `multiple` is toggled.
@@ -212,10 +207,7 @@ function _handleChange(event) {
     returnValue = props.onChange.call(undefined, event);
   }
 
-  if (this._rootNodeID) {
-    this._wrapperState.pendingUpdate = true;
-  }
-  ReactUpdates.asap(updateOptionsIfPendingUpdateAndMounted, this);
+  ReactUpdates.asap(forceUpdateIfMounted, this);
   return returnValue;
 }