@samitouri / QOS-React-2 / commits / 808a54f410

Batch subupdates caused by any state update

With this, multiple setState calls triggered by a componentDidUpdate handler (or similar) will be batched together, regardless of if the original setState call was in a batching context. I also cleaned up some inconsistencies with the order of component updates and callbacks in situations where one component's update directly causes another to update. Fixes #1147. Helps with #1353 and #1245 as well, though doesn't completely fix them yet. Test Plan: grunt test

Ben Alpert committed Apr 6, 2014 at 16:46 UTC 808a54f410af73e993982a536bfb220ee8cbe1a7
4 files changed +257 -38
src/addons/ReactRAFBatchingStrategy.js
+2 -2
@@ -36,8 +36,8 @@ var ReactRAFBatchingStrategy = {
36 * Call the provided function in a context within which calls to `setState`
37 * and friends are batched such that components aren't updated unnecessarily.
38 */
39 - batchedUpdates: function(callback, param) {
40 - callback(param);
39 + batchedUpdates: function(callback, a, b) {
40 + callback(a, b);
41 }
42 };
43
src/core/ReactDefaultBatchingStrategy.js
+3 -3
@@ -58,16 +58,16 @@ var ReactDefaultBatchingStrategy = {
58 * Call the provided function in a context within which calls to `setState`
59 * and friends are batched such that components aren't updated unnecessarily.
60 */
61 - batchedUpdates: function(callback, param) {
61 + batchedUpdates: function(callback, a, b) {
62 var alreadyBatchingUpdates = ReactDefaultBatchingStrategy.isBatchingUpdates;
63
64 ReactDefaultBatchingStrategy.isBatchingUpdates = true;
65
66 // The code is written this way to avoid extra allocations
67 if (alreadyBatchingUpdates) {
68 - callback(param);
68 + callback(a, b);
69 } else {
70 - transaction.perform(callback, null, param);
70 + transaction.perform(callback, null, a, b);
71 }
72 }
73 };
src/core/ReactUpdates.js
+94 -33
@@ -18,10 +18,14 @@
18
19 "use strict";
20
21 +var CallbackQueue = require('CallbackQueue');
22 +var PooledClass = require('PooledClass');
23 var ReactCurrentOwner = require('ReactCurrentOwner');
24 var ReactPerf = require('ReactPerf');
25 +var Transaction = require('Transaction');
26
27 var invariant = require('invariant');
28 +var mixInto = require('mixInto');
29 var warning = require('warning');
30
31 var dirtyComponents = [];
@@ -36,9 +40,77 @@ function ensureInjected() {
40 );
41 }
42
39 -function batchedUpdates(callback, param) {
43 +var NESTED_UPDATES = {
44 + initialize: function() {
45 + this.dirtyComponentsLength = dirtyComponents.length;
46 + },
47 + close: function() {
48 + if (this.dirtyComponentsLength !== dirtyComponents.length) {
49 + // Additional updates were enqueued by componentDidUpdate handlers or
50 + // similar; before our own UPDATE_QUEUEING wrapper closes, we want to run
51 + // these new updates so that if A's componentDidUpdate calls setState on
52 + // B, B will update before the callback A's updater provided when calling
53 + // setState.
54 + dirtyComponents.splice(0, this.dirtyComponentsLength);
55 + flushBatchedUpdates();
56 + } else {
57 + dirtyComponents.length = 0;
58 + }
59 + }
60 +};
61 +
62 +var UPDATE_QUEUEING = {
63 + initialize: function() {
64 + this.callbackQueue.reset();
65 + },
66 + close: function() {
67 + this.callbackQueue.notifyAll();
68 + }
69 +};
70 +
71 +var TRANSACTION_WRAPPERS = [NESTED_UPDATES, UPDATE_QUEUEING];
72 +
73 +function ReactUpdatesFlushTransaction() {
74 + this.reinitializeTransaction();
75 + this.dirtyComponentsLength = null;
76 + this.callbackQueue = CallbackQueue.getPooled(null);
77 + this.reconcileTransaction =
78 + ReactUpdates.ReactReconcileTransaction.getPooled();
79 +}
80 +
81 +mixInto(ReactUpdatesFlushTransaction, Transaction.Mixin);
82 +mixInto(ReactUpdatesFlushTransaction, {
83 + getTransactionWrappers: function() {
84 + return TRANSACTION_WRAPPERS;
85 + },
86 +
87 + destructor: function() {
88 + this.dirtyComponentsLength = null;
89 + CallbackQueue.release(this.callbackQueue);
90 + this.callbackQueue = null;
91 + ReactUpdates.ReactReconcileTransaction.release(this.reconcileTransaction);
92 + this.reconcileTransaction = null;
93 + },
94 +
95 + perform: function(method, scope, a) {
96 + // Essentially calls `this.reconcileTransaction.perform(method, scope, a)`
97 + // with this transaction's wrappers around it.
98 + return Transaction.Mixin.perform.call(
99 + this,
100 + this.reconcileTransaction.perform,
101 + this.reconcileTransaction,
102 + method,
103 + scope,
104 + a
105 + );
106 + }
107 +});
108 +
109 +PooledClass.addPoolingTo(ReactUpdatesFlushTransaction);
110 +
111 +function batchedUpdates(callback, a, b) {
112 ensureInjected();
41 - batchingStrategy.batchedUpdates(callback, param);
113 + batchingStrategy.batchedUpdates(callback, a, b);
114 }
115
116 /**
@@ -53,13 +125,21 @@ function mountDepthComparator(c1, c2) {
125 }
126
127 function runBatchedUpdates(transaction) {
128 + var len = transaction.dirtyComponentsLength;
129 + invariant(
130 + len === dirtyComponents.length,
131 + 'Expected flush transaction\'s stored dirty-components length (%s) to ' +
132 + 'match dirty-components array length (%s).',
133 + len,
134 + dirtyComponents.length
135 + );
136 +
137 // Since reconciling a component higher in the owner hierarchy usually (not
138 // always -- see shouldComponentUpdate()) will reconcile children, reconcile
139 // them before their children by sorting the array.
59 -
140 dirtyComponents.sort(mountDepthComparator);
141
62 - for (var i = 0; i < dirtyComponents.length; i++) {
142 + for (var i = 0; i < len; i++) {
143 // If a component is unmounted before pending changes apply, ignore them
144 // TODO: Queue unmounts in the same list to avoid this happening at all
145 var component = dirtyComponents[i];
@@ -69,11 +149,11 @@ function runBatchedUpdates(transaction) {
149 // stash the callbacks first
150 var callbacks = component._pendingCallbacks;
151 component._pendingCallbacks = null;
72 - component.performUpdateIfNecessary(transaction);
152 + component.performUpdateIfNecessary(transaction.reconcileTransaction);
153
154 if (callbacks) {
155 for (var j = 0; j < callbacks.length; j++) {
76 - transaction.getReactMountReady().enqueue(
156 + transaction.callbackQueue.enqueue(
157 callbacks[j],
158 component
159 );
@@ -83,30 +163,18 @@ function runBatchedUpdates(transaction) {
163 }
164 }
165
86 -function clearDirtyComponents() {
87 - dirtyComponents.length = 0;
88 -}
89 -
90 -function flushBatchedUpdatesOnce(transaction) {
91 - // Run these in separate functions so the JIT can optimize
92 - try {
93 - runBatchedUpdates(transaction);
94 - } finally {
95 - clearDirtyComponents();
96 - }
97 -}
98 -
166 var flushBatchedUpdates = ReactPerf.measure(
167 'ReactUpdates',
168 'flushBatchedUpdates',
169 function() {
103 - // flushBatchedUpdatesOnce will clear the dirtyComponents array, but
104 - // mount-ready handlers (i.e., componentDidMount/Update) may enqueue more
105 - // state updates which we should apply immediately
170 + // ReactUpdatesFlushTransaction's wrappers will clear the dirtyComponents
171 + // array and perform any updates enqueued by mount-ready handlers (i.e.,
172 + // componentDidUpdate) but we need to check here too in order to catch
173 + // updates enqueued by setState callbacks.
174 while (dirtyComponents.length) {
107 - var transaction = ReactUpdates.ReactReconcileTransaction.getPooled();
108 - transaction.perform(flushBatchedUpdatesOnce, null, transaction);
109 - ReactUpdates.ReactReconcileTransaction.release(transaction);
175 + var transaction = ReactUpdatesFlushTransaction.getPooled();
176 + transaction.perform(runBatchedUpdates, null, transaction);
177 + ReactUpdatesFlushTransaction.release(transaction);
178 }
179 }
180 );
@@ -138,14 +206,7 @@ function enqueueUpdate(component, callback) {
206 );
207
208 if (!batchingStrategy.isBatchingUpdates) {
141 - var transaction = ReactUpdates.ReactReconcileTransaction.getPooled();
142 - transaction.perform(
143 - component.performUpdateIfNecessary,
144 - component,
145 - transaction
146 - );
147 - ReactUpdates.ReactReconcileTransaction.release(transaction);
148 - callback && callback.call(component);
209 + batchingStrategy.batchedUpdates(enqueueUpdate, component, callback);
210 return;
211 }
212
src/core/__tests__/ReactUpdates-test.js
+158
@@ -565,4 +565,162 @@ describe('ReactUpdates', function() {
565
566 expect(aUpdated).toBe(true);
567 });
568 +
569 + it('should flush updates in the correct order', function() {
570 + var updates = [];
571 + var Outer = React.createClass({
572 + getInitialState: function() {
573 + return {x: 0};
574 + },
575 + render: function() {
576 + updates.push('Outer-render-' + this.state.x);
577 + return <div><Inner x={this.state.x} ref="inner" /></div>;
578 + },
579 + componentDidUpdate: function() {
580 + var x = this.state.x;
581 + updates.push('Outer-didUpdate-' + x);
582 + updates.push('Inner-setState-' + x);
583 + this.refs.inner.setState({x: x}, function() {
584 + updates.push('Inner-callback-' + x);
585 + });
586 + }
587 + });
588 + var Inner = React.createClass({
589 + getInitialState: function() {
590 + return {x: 0};
591 + },
592 + render: function() {
593 + updates.push('Inner-render-' + this.props.x + '-' + this.state.x);
594 + return <div />;
595 + },
596 + componentDidUpdate: function() {
597 + updates.push('Inner-didUpdate-' + this.props.x + '-' + this.state.x);
598 + }
599 + });
600 +
601 + var instance = ReactTestUtils.renderIntoDocument(<Outer />);
602 +
603 + updates.push('Outer-setState-1');
604 + instance.setState({x: 1}, function() {
605 + updates.push('Outer-callback-1');
606 + updates.push('Outer-setState-2');
607 + instance.setState({x: 2}, function() {
608 + updates.push('Outer-callback-2');
609 + });
610 + });
611 +
612 + expect(updates).toEqual([
613 + 'Outer-render-0',
614 + 'Inner-render-0-0',
615 +
616 + 'Outer-setState-1',
617 + 'Outer-render-1',
618 + 'Inner-render-1-0',
619 + 'Inner-didUpdate-1-0',
620 + 'Outer-didUpdate-1',
621 + 'Inner-setState-1',
622 + 'Inner-render-1-1',
623 + 'Inner-didUpdate-1-1',
624 + 'Inner-callback-1',
625 + 'Outer-callback-1',
626 +
627 + 'Outer-setState-2',
628 + 'Outer-render-2',
629 + 'Inner-render-2-1',
630 + 'Inner-didUpdate-2-1',
631 + 'Outer-didUpdate-2',
632 + 'Inner-setState-2',
633 + 'Inner-render-2-2',
634 + 'Inner-didUpdate-2-2',
635 + 'Inner-callback-2',
636 + 'Outer-callback-2'
637 + ]);
638 + });
639 +
640 + it('should queue nested updates', function() {
641 + // See https://github.com/facebook/react/issues/1147
642 +
643 + var X = React.createClass({
644 + getInitialState: function() {
645 + return {s: 0};
646 + },
647 + render: function() {
648 + if (this.state.s === 0) {
649 + return <div>
650 + <span>0</span>
651 + </div>;
652 + } else {
653 + return <div>1</div>;
654 + }
655 + },
656 + go: function() {
657 + this.setState({s: 1});
658 + this.setState({s: 0});
659 + this.setState({s: 1});
660 + }
661 + });
662 +
663 + var Y = React.createClass({
664 + render: function() {
665 + return <div>
666 + <Z />
667 + </div>;
668 + }
669 + });
670 +
671 + var Z = React.createClass({
672 + render: function() { return <div />; },
673 + componentWillUpdate: function() {
674 + x.go();
675 + }
676 + });
677 +
678 + var x;
679 + var y;
680 +
681 + x = ReactTestUtils.renderIntoDocument(<X />);
682 + y = ReactTestUtils.renderIntoDocument(<Y />);
683 + expect(x.getDOMNode().textContent).toBe('0');
684 +
685 + y.forceUpdate();
686 + expect(x.getDOMNode().textContent).toBe('1');
687 + });
688 +
689 + it('should queue updates from during mount', function() {
690 + // See https://github.com/facebook/react/issues/1353
691 + var a;
692 +
693 + var A = React.createClass({
694 + getInitialState: function() {
695 + return {x: 0};
696 + },
697 + componentWillMount: function() {
698 + a = this;
699 + },
700 + render: function() {
701 + return <div>A{this.state.x}</div>;
702 + }
703 + });
704 +
705 + var B = React.createClass({
706 + componentWillMount: function() {
707 + a.setState({x: 1});
708 + },
709 + render: function() {
710 + return <div />;
711 + }
712 + });
713 +
714 + ReactUpdates.batchedUpdates(function() {
715 + ReactTestUtils.renderIntoDocument(
716 + <div>
717 + <A />
718 + <B />
719 + </div>
720 + );
721 + });
722 +
723 + expect(a.state.x).toBe(1);
724 + expect(a.getDOMNode().textContent).toBe('A1');
725 + });
726 });