diff --git a/src/renderers/shared/reconciler/ReactCompositeComponent.js b/src/renderers/shared/reconciler/ReactCompositeComponent.js index 0b5982e3496ab..f1835938b968a 100644 --- a/src/renderers/shared/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/reconciler/ReactCompositeComponent.js @@ -153,6 +153,7 @@ var ReactCompositeComponentMixin = { this._nativeContainerInfo = null; // See ReactUpdateQueue + this._updateBatchNumber = null; this._pendingElement = null; this._pendingStateQueue = null; this._pendingReplaceState = false; @@ -765,9 +766,7 @@ var ReactCompositeComponentMixin = { transaction, this._context ); - } - - if (this._pendingStateQueue !== null || this._pendingForceUpdate) { + } else if (this._pendingStateQueue !== null || this._pendingForceUpdate) { this.updateComponent( transaction, this._currentElement, @@ -775,6 +774,8 @@ var ReactCompositeComponentMixin = { this._context, this._context ); + } else { + this._updateBatchNumber = null; } }, @@ -878,6 +879,7 @@ var ReactCompositeComponentMixin = { ); } + this._updateBatchNumber = null; if (shouldUpdate) { this._pendingForceUpdate = false; // Will set `this.props`, `this.state` and `this.context`. diff --git a/src/renderers/shared/reconciler/ReactReconciler.js b/src/renderers/shared/reconciler/ReactReconciler.js index cef23ca8506fa..cf2bfad093e10 100644 --- a/src/renderers/shared/reconciler/ReactReconciler.js +++ b/src/renderers/shared/reconciler/ReactReconciler.js @@ -14,6 +14,8 @@ var ReactRef = require('ReactRef'); var ReactInstrumentation = require('ReactInstrumentation'); +var invariant = require('invariant'); + /** * Helper to call ReactRef.attachRefs with this composite component, split out * to avoid allocations in the transaction mount-ready queue. @@ -190,8 +192,22 @@ var ReactReconciler = { */ performUpdateIfNecessary: function( internalInstance, - transaction + transaction, + updateBatchNumber ) { + if (internalInstance._updateBatchNumber !== updateBatchNumber) { + // The component's enqueued batch number should always be the current + // batch or the following one. + invariant( + internalInstance._updateBatchNumber == null || + internalInstance._updateBatchNumber === updateBatchNumber + 1, + 'performUpdateIfNecessary: Unexpected batch number (current %s, ' + + 'pending %s)', + updateBatchNumber, + internalInstance._updateBatchNumber + ); + return; + } if (__DEV__) { if (internalInstance._debugID !== 0) { ReactInstrumentation.debugTool.onBeginReconcilerTimer( diff --git a/src/renderers/shared/reconciler/ReactUpdates.js b/src/renderers/shared/reconciler/ReactUpdates.js index ff593be663984..225d7c50f89ab 100644 --- a/src/renderers/shared/reconciler/ReactUpdates.js +++ b/src/renderers/shared/reconciler/ReactUpdates.js @@ -22,6 +22,7 @@ var Transaction = require('Transaction'); var invariant = require('invariant'); var dirtyComponents = []; +var updateBatchNumber = 0; var asapCallbackQueue = CallbackQueue.getPooled(); var asapEnqueued = false; @@ -138,6 +139,13 @@ function runBatchedUpdates(transaction) { // them before their children by sorting the array. dirtyComponents.sort(mountOrderComparator); + // Any updates enqueued while reconciling must be performed after this entire + // batch. Otherwise, if dirtyComponents is [A, B] where A has children B and + // C, B could update twice in a single batch if C's render enqueues an update + // to B (since B would have already updated, we should skip it, and the only + // way we can know to do so is by checking the batch counter). + updateBatchNumber++; + for (var i = 0; i < len; i++) { // If a component is unmounted before pending changes apply, it will still // be here, but we assume that it has cleared its _pendingCallbacks and @@ -166,7 +174,8 @@ function runBatchedUpdates(transaction) { ReactReconciler.performUpdateIfNecessary( component, - transaction.reconcileTransaction + transaction.reconcileTransaction, + updateBatchNumber ); if (markerName) { @@ -238,6 +247,9 @@ function enqueueUpdate(component) { } dirtyComponents.push(component); + if (component._updateBatchNumber == null) { + component._updateBatchNumber = updateBatchNumber + 1; + } } /** diff --git a/src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js b/src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js index f59193d691ca4..96ff9d1835033 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js @@ -1024,4 +1024,102 @@ describe('ReactUpdates', function() { 'to be a function. Instead received: Foo (keys: a, b).' ); }); + + it('does not update one component twice in a batch (#2410)', function() { + var Parent = React.createClass({ + getChild: function() { + return this.refs.child; + }, + render: function() { + return ; + }, + }); + + var renderCount = 0; + var postRenderCount = 0; + var once = false; + var Child = React.createClass({ + getInitialState: function() { + return {updated: false}; + }, + componentWillUpdate: function() { + if (!once) { + once = true; + this.setState({updated: true}); + } + }, + componentDidMount: function() { + expect(renderCount).toBe(postRenderCount + 1); + postRenderCount++; + }, + componentDidUpdate: function() { + expect(renderCount).toBe(postRenderCount + 1); + postRenderCount++; + }, + render: function() { + expect(renderCount).toBe(postRenderCount); + renderCount++; + return
; + }, + }); + + var parent = ReactTestUtils.renderIntoDocument(); + var child = parent.getChild(); + ReactDOM.unstable_batchedUpdates(function() { + parent.forceUpdate(); + child.forceUpdate(); + }); + }); + + it('does not update one component twice in a batch (#6371)', function() { + var callbacks = []; + function emitChange() { + callbacks.forEach(c => c()); + } + + class App extends React.Component { + constructor(props) { + super(props); + this.state = { showChild: true }; + } + componentDidMount() { + console.log('about to remove child via set state'); + this.setState({ showChild: false }); + } + render() { + return ( +
+ + {this.state.showChild && } +
+ ); + } + } + + class EmitsChangeOnUnmount extends React.Component { + componentWillUnmount() { + emitChange(); + } + render() { + return null; + } + } + + class ForceUpdatesOnChange extends React.Component { + componentDidMount() { + this.onChange = () => this.forceUpdate(); + this.onChange(); + callbacks.push(this.onChange); + } + componentWillUnmount() { + callbacks = callbacks.filter((c) => c !== this.onChange); + } + render() { + return
; + } + } + + ReactDOM.render(, document.createElement('div')); + }); + });