diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 7955d43a141..bdb71eeb05d 100644 --- a/scripts/fiber/tests-failing.txt +++ b/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` diff --git a/src/renderers/dom/shared/ReactEventListener.js b/src/renderers/dom/shared/ReactEventListener.js index 74dc16ddd13..44177f69ae9 100644 --- a/src/renderers/dom/shared/ReactEventListener.js +++ b/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); } diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index f84e2545bb4..2bab859aa8b 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/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) { diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index d61b5cca2be..5c861012665 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/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 ( +
+ this.a = n} + value={this.state.value} + onChange={e => this.change(e.target.value)} + onBlur={e => this.blur(e.target.value)} + /> + this.b = n} + /> +
+ ); + } + } + + var container = document.createElement('div'); + var instance = ReactDOM.render(, 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 = ; stub = ReactTestUtils.renderIntoDocument(stub); diff --git a/src/renderers/shared/shared/event/ReactGenericBatching.js b/src/renderers/shared/shared/event/ReactGenericBatching.js index 6461ae973db..9fdab98daa4 100644 --- a/src/renderers/shared/shared/event/ReactGenericBatching.js +++ b/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, };