From ddf53f97f09a652f173bc509b903ead920d92b52 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Sat, 5 Apr 2014 18:13:22 -0700 Subject: [PATCH] Share reconcile transaction in batched updates Should make #1350 better and will also take away any performance hit from #1157. Test Plan: grunt test --- src/browser/ui/ReactDefaultInjection.js | 3 ++ src/core/ReactComponent.js | 29 ++-------- src/core/ReactCompositeComponent.js | 21 ++++---- src/core/ReactUpdates.js | 67 ++++++++++++++++++----- src/core/__tests__/ReactUpdates-test.js | 72 +++++++++++++++++++++++++ 5 files changed, 142 insertions(+), 50 deletions(-) diff --git a/src/browser/ui/ReactDefaultInjection.js b/src/browser/ui/ReactDefaultInjection.js index 9724c6c63d..7a63a75c94 100644 --- a/src/browser/ui/ReactDefaultInjection.js +++ b/src/browser/ui/ReactDefaultInjection.js @@ -103,6 +103,9 @@ function inject() { ReactInjection.EmptyComponent.injectEmptyComponent(ReactDOM.script); + ReactInjection.Updates.injectReconcileTransaction( + ReactComponentBrowserEnvironment.ReactReconcileTransaction + ); ReactInjection.Updates.injectBatchingStrategy( ReactDefaultBatchingStrategy ); diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 2d6acf1974..b2659a2075 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -101,8 +101,6 @@ var ReactComponent = { ReactComponentEnvironment.unmountIDFromEnvironment; ReactComponent.BackendIDOperations = ReactComponentEnvironment.BackendIDOperations; - ReactComponent.ReactReconcileTransaction = - ReactComponentEnvironment.ReactReconcileTransaction; injected = true; } }, @@ -121,14 +119,6 @@ var ReactComponent = { */ BackendIDOperations: null, - /** - * React references `ReactReconcileTransaction` using this property in order - * to allow dependency injection. - * - * @internal - */ - ReactReconcileTransaction: null, - /** * Base functionality for every ReactComponent constructor. Mixed into the * `ReactComponent` prototype, but exposed statically for easy access. @@ -321,18 +311,7 @@ var ReactComponent = { 'receiveComponent(...): Can only update a mounted component.' ); this._pendingDescriptor = nextDescriptor; - this._performUpdateIfNecessary(transaction); - }, - - /** - * Call `_performUpdateIfNecessary` within a new transaction. - * - * @internal - */ - performUpdateIfNecessary: function() { - var transaction = ReactComponent.ReactReconcileTransaction.getPooled(); - transaction.perform(this._performUpdateIfNecessary, this, transaction); - ReactComponent.ReactReconcileTransaction.release(transaction); + this.performUpdateIfNecessary(transaction); }, /** @@ -341,7 +320,7 @@ var ReactComponent = { * @param {ReactReconcileTransaction} transaction * @internal */ - _performUpdateIfNecessary: function(transaction) { + performUpdateIfNecessary: function(transaction) { if (this._pendingDescriptor == null) { return; } @@ -405,7 +384,7 @@ var ReactComponent = { * @see {ReactMount.renderComponent} */ mountComponentIntoNode: function(rootID, container, shouldReuseMarkup) { - var transaction = ReactComponent.ReactReconcileTransaction.getPooled(); + var transaction = ReactUpdates.ReactReconcileTransaction.getPooled(); transaction.perform( this._mountComponentIntoNode, this, @@ -414,7 +393,7 @@ var ReactComponent = { transaction, shouldReuseMarkup ); - ReactComponent.ReactReconcileTransaction.release(transaction); + ReactUpdates.ReactReconcileTransaction.release(transaction); }, /** diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 5144319476..ec3178b8ec 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -932,17 +932,6 @@ var ReactCompositeComponentMixin = { } }, - performUpdateIfNecessary: function() { - var compositeLifeCycleState = this._compositeLifeCycleState; - // Do not trigger a state transition if we are in the middle of mounting or - // receiving props because both of those will already be doing this. - if (compositeLifeCycleState === CompositeLifeCycle.MOUNTING || - compositeLifeCycleState === CompositeLifeCycle.RECEIVING_PROPS) { - return; - } - ReactComponent.Mixin.performUpdateIfNecessary.call(this); - }, - /** * If any of `_pendingDescriptor`, `_pendingState`, or `_pendingForceUpdate` * is set, update the component. @@ -950,7 +939,15 @@ var ReactCompositeComponentMixin = { * @param {ReactReconcileTransaction} transaction * @internal */ - _performUpdateIfNecessary: function(transaction) { + performUpdateIfNecessary: function(transaction) { + var compositeLifeCycleState = this._compositeLifeCycleState; + // Do not trigger a state transition if we are in the middle of mounting or + // receiving props because both of those will already be doing this. + if (compositeLifeCycleState === CompositeLifeCycle.MOUNTING || + compositeLifeCycleState === CompositeLifeCycle.RECEIVING_PROPS) { + return; + } + if (this._pendingDescriptor == null && this._pendingState == null && !this._pendingForceUpdate) { diff --git a/src/core/ReactUpdates.js b/src/core/ReactUpdates.js index d1d053785e..d59aa10e9d 100644 --- a/src/core/ReactUpdates.js +++ b/src/core/ReactUpdates.js @@ -26,12 +26,16 @@ var dirtyComponents = []; var batchingStrategy = null; -function ensureBatchingStrategy() { - invariant(batchingStrategy, 'ReactUpdates: must inject a batching strategy'); +function ensureInjected() { + invariant( + ReactUpdates.ReactReconcileTransaction && batchingStrategy, + 'ReactUpdates: must inject a reconcile transaction class and batching ' + + 'strategy' + ); } function batchedUpdates(callback, param) { - ensureBatchingStrategy(); + ensureInjected(); batchingStrategy.batchedUpdates(callback, param); } @@ -46,7 +50,7 @@ function mountDepthComparator(c1, c2) { return c1._mountDepth - c2._mountDepth; } -function runBatchedUpdates() { +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. @@ -63,10 +67,14 @@ function runBatchedUpdates() { // stash the callbacks first var callbacks = component._pendingCallbacks; component._pendingCallbacks = null; - component.performUpdateIfNecessary(); + component.performUpdateIfNecessary(transaction); + if (callbacks) { for (var j = 0; j < callbacks.length; j++) { - callbacks[j].call(component); + transaction.getReactMountReady().enqueue( + callbacks[j], + component + ); } } } @@ -77,15 +85,26 @@ function clearDirtyComponents() { dirtyComponents.length = 0; } +function flushBatchedUpdatesOnce(transaction) { + // Run these in separate functions so the JIT can optimize + try { + runBatchedUpdates(transaction); + } finally { + clearDirtyComponents(); + } +} + var flushBatchedUpdates = ReactPerf.measure( 'ReactUpdates', 'flushBatchedUpdates', function() { - // Run these in separate functions so the JIT can optimize - try { - runBatchedUpdates(); - } finally { - clearDirtyComponents(); + // flushBatchedUpdatesOnce will clear the dirtyComponents array, but + // mount-ready handlers (i.e., componentDidMount/Update) may enqueue more + // state updates which we should apply immediately + while (dirtyComponents.length) { + var transaction = ReactUpdates.ReactReconcileTransaction.getPooled(); + transaction.perform(flushBatchedUpdatesOnce, null, transaction); + ReactUpdates.ReactReconcileTransaction.release(transaction); } } ); @@ -101,10 +120,16 @@ function enqueueUpdate(component, callback) { '`setState`, `replaceState`, or `forceUpdate` with a callback that ' + 'isn\'t callable.' ); - ensureBatchingStrategy(); + ensureInjected(); if (!batchingStrategy.isBatchingUpdates) { - component.performUpdateIfNecessary(); + var transaction = ReactUpdates.ReactReconcileTransaction.getPooled(); + transaction.perform( + component.performUpdateIfNecessary, + component, + transaction + ); + ReactUpdates.ReactReconcileTransaction.release(transaction); callback && callback.call(component); return; } @@ -121,6 +146,14 @@ function enqueueUpdate(component, callback) { } var ReactUpdatesInjection = { + injectReconcileTransaction: function(ReconcileTransaction) { + invariant( + ReconcileTransaction, + 'ReactUpdates: must provide a reconcile transaction class' + ); + ReactUpdates.ReactReconcileTransaction = ReconcileTransaction; + }, + injectBatchingStrategy: function(_batchingStrategy) { invariant( _batchingStrategy, @@ -139,6 +172,14 @@ var ReactUpdatesInjection = { }; var ReactUpdates = { + /** + * React references `ReactReconcileTransaction` using this property in order + * to allow dependency injection. + * + * @internal + */ + ReactReconcileTransaction: null, + batchedUpdates: batchedUpdates, enqueueUpdate: enqueueUpdate, flushBatchedUpdates: flushBatchedUpdates, diff --git a/src/core/__tests__/ReactUpdates-test.js b/src/core/__tests__/ReactUpdates-test.js index 690af3b443..8ff3eff48b 100644 --- a/src/core/__tests__/ReactUpdates-test.js +++ b/src/core/__tests__/ReactUpdates-test.js @@ -493,4 +493,76 @@ describe('ReactUpdates', function() { ['Box', 'Switcher', 'Child'] ); }); + + it('should share reconcile transaction across different roots', function() { + var ReconcileTransaction = ReactUpdates.ReactReconcileTransaction; + spyOn(ReconcileTransaction, 'getPooled').andCallThrough(); + + var Component = React.createClass({ + render: function() { + return
{this.props.text}
; + } + }); + + var containerA = document.createElement('div'); + var containerB = document.createElement('div'); + + // Initial renders aren't batched together yet... + ReactUpdates.batchedUpdates(function() { + React.renderComponent(, containerA); + React.renderComponent(, containerB); + }); + expect(ReconcileTransaction.getPooled.calls.length).toBe(2); + + // ...but updates are! Here only one more transaction is used, which means + // we only have to initialize and close the wrappers once. + ReactUpdates.batchedUpdates(function() { + React.renderComponent(, containerA); + React.renderComponent(, containerB); + }); + expect(ReconcileTransaction.getPooled.calls.length).toBe(3); + }); + + it('should queue mount-ready handlers across different roots', function() { + // We'll define two components A and B, then update both of them. When A's + // componentDidUpdate handlers is called, B's DOM should already have been + // updated. + + var a; + var b; + + var aUpdated = false; + + var A = React.createClass({ + getInitialState: function() { + return {x: 0}; + }, + componentDidUpdate: function() { + expect(b.getDOMNode().textContent).toBe("B1"); + aUpdated = true; + }, + render: function() { + return
A{this.state.x}
; + } + }); + + var B = React.createClass({ + getInitialState: function() { + return {x: 0}; + }, + render: function() { + return
B{this.state.x}
; + } + }); + + a = ReactTestUtils.renderIntoDocument(); + b = ReactTestUtils.renderIntoDocument(); + + ReactUpdates.batchedUpdates(function() { + a.setState({x: 1}); + b.setState({x: 1}); + }); + + expect(aUpdated).toBe(true); + }); });