Fixed batching reentrant controlled events

scripts/fiber/tests-failing.txt

@@ -250,6 +250,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js
 
 src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
 * should properly control a value even if no event listener exists
+* should control a value in reentrant events
 * should display `defaultValue` of number 0
 * should display "true" for `defaultValue` of `true`
 * should display "false" for `defaultValue` of `false`

src/renderers/dom/shared/ReactEventListener.js

@@ -14,7 +14,6 @@
 var EventListener = require('EventListener');
 var ExecutionEnvironment = require('ExecutionEnvironment');
 var PooledClass = require('PooledClass');
-var ReactControlledComponent = require('ReactControlledComponent');
 var ReactDOMComponentTree = require('ReactDOMComponentTree');
 var ReactGenericBatching = require('ReactGenericBatching');
 
@@ -172,14 +171,11 @@ var ReactEventListener = {
     try {
       // Event queue being processed in the same cycle allows
       // `preventDefault`.
-      ReactGenericBatching.batchedUpdates(handleTopLevelImpl, bookKeeping);
-      if (bookKeeping.targetInst) {
-        // Here we wait until all updates have propagated, which is important
-        // when using controlled components within layers:
-        // https://github.com/facebook/react/issues/1698
-        // Then we restore state of any controlled component.
-        ReactControlledComponent.restoreStateIfNeeded(bookKeeping.targetInst);
-      }
+      ReactGenericBatching.batchedUpdatesWithControlledTarget(
+        handleTopLevelImpl,
+        bookKeeping,
+        bookKeeping.targetInst
+      );
     } finally {
       TopLevelCallbackBookKeeping.release(bookKeeping);
     }

src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js

@@ -101,10 +101,11 @@ function manualDispatchChangeEvent(nativeEvent) {
   // components don't work properly in conjunction with event bubbling because
   // the component is rerendered and the value reverted before all the event
   // handlers can run. See https://github.com/facebook/react/issues/708.
-  ReactGenericBatching.batchedUpdates(runEventInBatch, event);
-  if (activeElementInst) {
-    ReactControlledComponent.restoreStateIfNeeded(activeElementInst);
-  }
+  ReactGenericBatching.batchedUpdatesWithControlledTarget(
+    runEventInBatch,
+    event,
+    activeElementInst
+  );
 }
 
 function runEventInBatch(event) {

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js

@@ -66,6 +66,72 @@ describe('ReactDOMInput', () => {
     document.body.removeChild(container);
   });
 
+  it('should control a value in reentrant events', () => {
+    // This must use the native event dispatching. If we simulate, we will
+    // bypass the lazy event attachment system so we won't actually test this.
+    var inputEvent = document.createEvent('Event');
+    inputEvent.initEvent('input', true, true);
+    // This must use the native event dispatching. If we simulate, we will
+    // bypass the lazy event attachment system so we won't actually test this.
+    var changeEvent = document.createEvent('Event');
+    changeEvent.initEvent('change', true, true);
+
+    class ControlledInputs extends React.Component {
+      state = { value: 'lion' };
+      a = null;
+      b = null;
+      switchedFocus = false;
+      change(newValue) {
+        this.setState({ value: newValue });
+        // Calling focus here will blur the text box which causes a native
+        // change event. Ideally we shouldn't have to fire this ourselves.
+        // I don't know how to simulate a change event on a text box.
+        this.a.dispatchEvent(changeEvent);
+        this.b.focus();
+      }
+      blur(currentValue) {
+        this.switchedFocus = true;
+        // currentValue should be 'giraffe' here because we should not have
+        // restored it on the target yet.
+        this.setState({ value: currentValue });
+      }
+      render() {
+        return (
+          <div>
+            <input
+              type="text"
+              ref={n => this.a = n}
+              value={this.state.value}
+              onChange={e => this.change(e.target.value)}
+              onBlur={e => this.blur(e.target.value)}
+            />
+            <input
+              type="text"
+              ref={n => this.b = n}
+            />
+          </div>
+        );
+      }
+    }
+
+    var container = document.createElement('div');
+    var instance = ReactDOM.render(<ControlledInputs />, container);
+
+    // We need it to be in the body to test native event dispatching.
+    document.body.appendChild(container);
+
+    instance.a.focus();
+    // Simulate a native keyup event
+    setUntrackedValue(instance.a, 'giraffe');
+
+    instance.a.dispatchEvent(inputEvent);
+
+    expect(instance.a.value).toBe('giraffe');
+    expect(instance.switchedFocus).toBe(true);
+
+    document.body.removeChild(container);
+  });
+
   it('should display `defaultValue` of number 0', () => {
     var stub = <input type="text" defaultValue={0} />;
     stub = ReactTestUtils.renderIntoDocument(stub);

src/renderers/shared/shared/event/ReactGenericBatching.js

@@ -11,6 +11,8 @@
 
 'use strict';
 
+var ReactControlledComponent = require('ReactControlledComponent');
+
 // Used as a way to call batchedUpdates when we don't know if we're in a Fiber
 // or Stack context. Such as when we're dispatching events or if third party
 // libraries need to call batchedUpdates. Eventually, this API will go away when
@@ -36,6 +38,29 @@ function batchedUpdates(fn, bookkeeping) {
   stackBatchedUpdates(performFiberBatchedUpdates, fn, bookkeeping);
 }
 
+var isBatching = false;
+function batchedUpdatesWithControlledTarget(fn, bookkeeping, target) {
+  if (isBatching) {
+    // TODO: If this target is not the same as the one currently batched,
+    // we'll drop it.
+    batchedUpdates(fn, bookkeeping);
+    return;
+  }
+  isBatching = true;
+  try {
+    batchedUpdates(fn, bookkeeping);
+  } finally {
+    isBatching = false;
+  }
+  if (target) {
+    // Here we wait until all updates have propagated, which is important
+    // when using controlled components within layers:
+    // https://github.com/facebook/react/issues/1698
+    // Then we restore state of any controlled component.
+    ReactControlledComponent.restoreStateIfNeeded(target);
+  }
+}
+
 var ReactGenericBatchingInjection = {
   injectStackBatchedUpdates: function(_batchedUpdates) {
     stackBatchedUpdates = _batchedUpdates;
@@ -47,6 +72,7 @@ var ReactGenericBatchingInjection = {
 
 var ReactGenericBatching = {
   batchedUpdates,
+  batchedUpdatesWithControlledTarget,
   injection: ReactGenericBatchingInjection,
 };