Skip to content

Update Dirty Components in Mount Ordering #2570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yungsters merged 1 commit into from
Nov 19, 2014
Merged

Update Dirty Components in Mount Ordering #2570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/core/ReactCompositeComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,14 @@ var CompositeLifeCycle = keyMirror({
RECEIVING_PROPS: 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;

/**
* @lends {ReactCompositeComponent.prototype}
*/
Expand Down Expand Up @@ -132,6 +140,7 @@ var ReactCompositeComponentMixin = assign({},
ReactComponent.Mixin.construct.apply(this, arguments);

this._context = null;
this._mountOrder = 0;

// See ReactUpdates.
this._pendingCallbacks = null;
Expand Down Expand Up @@ -167,6 +176,7 @@ var ReactCompositeComponentMixin = assign({},
);

this._context = context;
this._mountOrder = nextMountID++;
this._rootNodeID = rootID;

var inst = this._instance;
Expand Down
8 changes: 4 additions & 4 deletions src/core/ReactUpdates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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, it will still
Expand Down
37 changes: 37 additions & 0 deletions src/core/__tests__/ReactUpdates-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,43 @@ 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.depth);
return <div />;
},
componentDidMount: function() {
instances.push(this);
if (this.props.depth < this.props.count) {
React.renderComponent(
<MockComponent
depth={this.props.depth + 1}
count={this.props.count}
/>,
this.getDOMNode()
);
}
}
});

ReactTestUtils.renderIntoDocument(<MockComponent depth={0} count={2} />);

expect(updates).toEqual([0, 1, 2]);

ReactUpdates.batchedUpdates(function() {
// Simulate update on each component from top to bottom.
instances.forEach(function(instance) {
instance.forceUpdate();
});
});

expect(updates).toEqual([0, 1, 2, 0, 1, 2]);
});

it('should queue nested updates', function() {
// See https://github.com/facebook/react/issues/1147

Expand Down