From 763ff4b13a0c11dd4a3788e4d9dc3a4056fde038 Mon Sep 17 00:00:00 2001 From: yungsters Date: Thu, 16 Oct 2014 14:27:04 -0700 Subject: [PATCH] Update Dirty Components in Mount Ordering Summary: Currently, `ReactUpdates` updates dirty components in increasing order of mount depth. However, mount depth is only relative to the component passed into `React.render`. This breaks down for components that invoke `React.render` as an implementation detail because the child components will be updated before the parent component. This fixes the problem by using the order in which components are mounted (instead of their depth). The mount order transcends component trees (rooted at `React.render` calls). Test Plan: Ran unit tests successfully: ``` npm run jest ``` --- src/core/ReactComponent.js | 9 +++++++ src/core/ReactUpdates.js | 8 +++--- src/core/__tests__/ReactUpdates-test.js | 33 +++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index bc9e0c01d51..ae0b7160dcf 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -56,6 +56,14 @@ var unmountIDFromEnvironment = null; */ var mountImageIntoNode = null; +/** + * An incrementing ID assigned to each component when it is mounted. This is + * used to enforce the order in which `ReactUpdates` updates dirty components. + * + * @private + */ +var nextMountID = 1; + /** * Components are the basic units of composition in React. * @@ -260,6 +268,7 @@ var ReactComponent = { this._rootNodeID = rootID; this._lifeCycleState = ComponentLifeCycle.MOUNTED; this._mountDepth = mountDepth; + this._mountOrder = nextMountID++; // Effectively: return ''; }, diff --git a/src/core/ReactUpdates.js b/src/core/ReactUpdates.js index 7486e3ac674..0cde3e9404e 100644 --- a/src/core/ReactUpdates.js +++ b/src/core/ReactUpdates.js @@ -110,14 +110,14 @@ function batchedUpdates(callback, a, b) { } /** - * Array comparator for ReactComponents by owner depth + * Array comparator for ReactComponents by mount ordering. * * @param {ReactComponent} c1 first component you're comparing * @param {ReactComponent} c2 second component you're comparing * @return {number} Return value usable by Array.prototype.sort(). */ -function mountDepthComparator(c1, c2) { - return c1._mountDepth - c2._mountDepth; +function mountOrderComparator(c1, c2) { + return c1._mountOrder - c2._mountOrder; } function runBatchedUpdates(transaction) { @@ -133,7 +133,7 @@ function runBatchedUpdates(transaction) { // Since reconciling a component higher in the owner hierarchy usually (not // always -- see shouldComponentUpdate()) will reconcile children, reconcile // them before their children by sorting the array. - dirtyComponents.sort(mountDepthComparator); + dirtyComponents.sort(mountOrderComparator); for (var i = 0; i < len; i++) { // If a component is unmounted before pending changes apply, ignore them diff --git a/src/core/__tests__/ReactUpdates-test.js b/src/core/__tests__/ReactUpdates-test.js index 57745600073..d09c60629a0 100644 --- a/src/core/__tests__/ReactUpdates-test.js +++ b/src/core/__tests__/ReactUpdates-test.js @@ -629,6 +629,39 @@ describe('ReactUpdates', function() { ]); }); + it('should flush updates in the correct order across roots', function() { + var instances = []; + var updates = []; + + var MockComponent = React.createClass({ + render: function() { + updates.push(this.props.count); + return
; + }, + componentDidMount: function() { + instances.push(this); + if (this.props.count) { + React.renderComponent( + , + this.getDOMNode() + ); + } + } + }); + + ReactTestUtils.renderIntoDocument(
); + + expect(updates).toEqual([2, 1, 0]); + + ReactUpdates.batchedUpdates(function() { + instances.forEach(function(instance) { + instance.forceUpdate(); + }); + }); + + expect(updates).toEqual([2, 1, 0, 2, 1, 0]); + }); + it('should queue nested updates', function() { // See https://github.com/facebook/react/issues/1147