@samitouri / QOS-React-2 / commits / c7fd626b1f

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.

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). Reviewers: @sebmarkbage @zpao Test Plan: Ran unit tests successfully: ``` npm run jest ```

yungsters committed Nov 4, 2014 at 14:25 UTC c7fd626b1f47d72d9b99fcc065c5ef44da3efcfc
3 files changed +51 -4
src/core/ReactCompositeComponent.js
+10
@@ -99,6 +99,14 @@ var CompositeLifeCycle = keyMirror({
99 RECEIVING_PROPS: null
100 });
101
102 +/**
103 + * An incrementing ID assigned to each component when it is mounted. This is
104 + * used to enforce the order in which `ReactUpdates` updates dirty components.
105 + *
106 + * @private
107 + */
108 +var nextMountID = 1;
109 +
110 /**
111 * @lends {ReactCompositeComponent.prototype}
112 */
@@ -132,6 +140,7 @@ var ReactCompositeComponentMixin = assign({},
140 ReactComponent.Mixin.construct.apply(this, arguments);
141
142 this._context = null;
143 + this._mountOrder = 0;
144
145 // See ReactUpdates.
146 this._pendingCallbacks = null;
@@ -167,6 +176,7 @@ var ReactCompositeComponentMixin = assign({},
176 );
177
178 this._context = context;
179 + this._mountOrder = nextMountID++;
180 this._rootNodeID = rootID;
181
182 var inst = this._instance;
src/core/ReactUpdates.js
+4 -4
@@ -110,14 +110,14 @@ function batchedUpdates(callback, a, b) {
110 }
111
112 /**
113 - * Array comparator for ReactComponents by owner depth
113 + * Array comparator for ReactComponents by mount ordering.
114 *
115 * @param {ReactComponent} c1 first component you're comparing
116 * @param {ReactComponent} c2 second component you're comparing
117 * @return {number} Return value usable by Array.prototype.sort().
118 */
119 -function mountDepthComparator(c1, c2) {
120 - return c1._mountDepth - c2._mountDepth;
119 +function mountOrderComparator(c1, c2) {
120 + return c1._mountOrder - c2._mountOrder;
121 }
122
123 function runBatchedUpdates(transaction) {
@@ -133,7 +133,7 @@ function runBatchedUpdates(transaction) {
133 // Since reconciling a component higher in the owner hierarchy usually (not
134 // always -- see shouldComponentUpdate()) will reconcile children, reconcile
135 // them before their children by sorting the array.
136 - dirtyComponents.sort(mountDepthComparator);
136 + dirtyComponents.sort(mountOrderComparator);
137
138 for (var i = 0; i < len; i++) {
139 // If a component is unmounted before pending changes apply, it will still
src/core/__tests__/ReactUpdates-test.js
+37
@@ -629,6 +629,43 @@ describe('ReactUpdates', function() {
629 ]);
630 });
631
632 + it('should flush updates in the correct order across roots', function() {
633 + var instances = [];
634 + var updates = [];
635 +
636 + var MockComponent = React.createClass({
637 + render: function() {
638 + updates.push(this.props.depth);
639 + return <div />;
640 + },
641 + componentDidMount: function() {
642 + instances.push(this);
643 + if (this.props.depth < this.props.count) {
644 + React.renderComponent(
645 + <MockComponent
646 + depth={this.props.depth + 1}
647 + count={this.props.count}
648 + />,
649 + this.getDOMNode()
650 + );
651 + }
652 + }
653 + });
654 +
655 + ReactTestUtils.renderIntoDocument(<MockComponent depth={0} count={2} />);
656 +
657 + expect(updates).toEqual([0, 1, 2]);
658 +
659 + ReactUpdates.batchedUpdates(function() {
660 + // Simulate update on each component from top to bottom.
661 + instances.forEach(function(instance) {
662 + instance.forceUpdate();
663 + });
664 + });
665 +
666 + expect(updates).toEqual([0, 1, 2, 0, 1, 2]);
667 + });
668 +
669 it('should queue nested updates', function() {
670 // See https://github.com/facebook/react/issues/1147
671