Skip to content
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.

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

Batch updates in initial render #2935

Merged
merged 1 commit into from
Jan 27, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/addons/ReactRAFBatchingStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ var ReactRAFBatchingStrategy = {
* Call the provided function in a context within which calls to `setState`
* and friends are batched such that components aren't updated unnecessarily.
*/
batchedUpdates: function(callback, a, b) {
callback(a, b);
batchedUpdates: function(callback, a, b, c, d) {
callback(a, b, c, d);
}
};

Expand Down
43 changes: 36 additions & 7 deletions src/browser/ui/ReactMount.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,23 +233,51 @@ function findDeepestCachedAncestor(targetID) {
/**
* Mounts this component and inserts it into the DOM.
*
* @param {ReactComponent} componentInstance The instance to mount.
* @param {string} rootID DOM ID of the root node.
* @param {DOMElement} container DOM element to mount into.
* @param {ReactReconcileTransaction} transaction
* @param {boolean} shouldReuseMarkup If true, do not insert markup
*/
function mountComponentIntoNode(
componentInstance,
rootID,
container,
transaction,
shouldReuseMarkup) {
var markup = ReactReconciler.mountComponent(
this, rootID, transaction, emptyObject
componentInstance, rootID, transaction, emptyObject
);
this._isTopLevel = true;
componentInstance._isTopLevel = true;
ReactMount._mountImageIntoNode(markup, container, shouldReuseMarkup);
}

/**
* Batched mount.
*
* @param {ReactComponent} componentInstance The instance to mount.
* @param {string} rootID DOM ID of the root node.
* @param {DOMElement} container DOM element to mount into.
* @param {boolean} shouldReuseMarkup If true, do not insert markup
*/
function batchedMountComponentIntoNode(
componentInstance,
rootID,
container,
shouldReuseMarkup) {
var transaction = ReactUpdates.ReactReconcileTransaction.getPooled();
transaction.perform(
mountComponentIntoNode,
null,
componentInstance,
rootID,
container,
transaction,
shouldReuseMarkup
);
ReactUpdates.ReactReconcileTransaction.release(transaction);
}

/**
* Mounting is the process of initializing a React component by creatings its
* representative DOM elements and inserting them into a supplied `container`.
Expand Down Expand Up @@ -368,16 +396,17 @@ var ReactMount = {
container
);

var transaction = ReactUpdates.ReactReconcileTransaction.getPooled();
transaction.perform(
mountComponentIntoNode,
// The initial render is synchronous but any updates that happen during
// rendering, in componentWillMount or componentDidMount, will be batched
// according to the current batching strategy.

ReactUpdates.batchedUpdates(
batchedMountComponentIntoNode,
componentInstance,
reactRootID,
container,
transaction,
shouldReuseMarkup
);
ReactUpdates.ReactReconcileTransaction.release(transaction);

if (__DEV__) {
// Record the root element in case it later gets transplanted.
Expand Down
6 changes: 3 additions & 3 deletions src/core/ReactDefaultBatchingStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,16 @@ var ReactDefaultBatchingStrategy = {
* Call the provided function in a context within which calls to `setState`
* and friends are batched such that components aren't updated unnecessarily.
*/
batchedUpdates: function(callback, a, b) {
batchedUpdates: function(callback, a, b, c, d) {
var alreadyBatchingUpdates = ReactDefaultBatchingStrategy.isBatchingUpdates;

ReactDefaultBatchingStrategy.isBatchingUpdates = true;

// The code is written this way to avoid extra allocations
if (alreadyBatchingUpdates) {
callback(a, b);
callback(a, b, c, d);
} else {
transaction.perform(callback, null, a, b);
transaction.perform(callback, null, a, b, c, d);
}
}
};
Expand Down
4 changes: 2 additions & 2 deletions src/core/ReactUpdates.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ assign(

PooledClass.addPoolingTo(ReactUpdatesFlushTransaction);

function batchedUpdates(callback, a, b) {
function batchedUpdates(callback, a, b, c, d) {
ensureInjected();
batchingStrategy.batchedUpdates(callback, a, b);
batchingStrategy.batchedUpdates(callback, a, b, c, d);
}

/**
Expand Down
8 changes: 3 additions & 5 deletions src/core/__tests__/ReactCompositeComponentState-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@ describe('ReactCompositeComponent-state', function() {
[ 'componentDidMount-start', 'orange', null ],
// setState-sunrise and setState-orange should be called here,
// after the bug in #1740
// componentDidMount() called setState({color:'yellow'}), currently this
// occurs inline.
// In a future where setState() is async, this test result will change.
// componentDidMount() called setState({color:'yellow'}), which is async.
// The update doesn't happen until the next flush.
[ 'componentDidMount-end', 'orange', 'yellow' ],
[ 'shouldComponentUpdate-currentState', 'orange', null ],
[ 'shouldComponentUpdate-nextState', 'yellow' ],
[ 'componentWillUpdate-currentState', 'orange', null ],
Expand All @@ -166,8 +166,6 @@ describe('ReactCompositeComponent-state', function() {
[ 'componentDidUpdate-currentState', 'yellow', null ],
[ 'componentDidUpdate-prevState', 'orange' ],
[ 'setState-yellow', 'yellow', null ],
// componentDidMount() finally closes.
[ 'componentDidMount-end', 'yellow', null ],
[ 'initial-callback', 'yellow', null ],
[ 'componentWillReceiveProps-start', 'yellow', null ],
// setState({color:'green'}) only enqueues a pending state.
Expand Down