@samitouri / QOS-React-2 / commits / 58de758a32

Sort batched updates by owner depth

If we reconcile components higher in the hierarchy they will likely reconcile components lower in the hierarchy. If we sort by depth then when we reach those components there will be no more pending state or props and it will no op.

Pete Hunt committed Oct 1, 2013 at 13:31 UTC 58de758a324ec75fb7ecf988dbbb645984c38857
11 files changed +317 -22
src/core/ReactComponent.js
+4 -2
@@ -335,10 +335,11 @@ var ReactComponent = {
335 *
336 * @param {string} rootID DOM ID of the root node.
337 * @param {ReactReconcileTransaction} transaction
338 + * @param {number} mountDepth number of components in the owner hierarchy.
339 * @return {?string} Rendered markup to be inserted into the DOM.
340 * @internal
341 */
341 - mountComponent: function(rootID, transaction) {
342 + mountComponent: function(rootID, transaction, mountDepth) {
343 invariant(
344 !this.isMounted(),
345 'mountComponent(%s, ...): Can only mount an unmounted component.',
@@ -350,6 +351,7 @@ var ReactComponent = {
351 }
352 this._rootNodeID = rootID;
353 this._lifeCycleState = ComponentLifeCycle.MOUNTED;
354 + this._mountDepth = mountDepth;
355 // Effectively: return '';
356 },
357
@@ -485,7 +487,7 @@ var ReactComponent = {
487 container,
488 transaction,
489 shouldReuseMarkup) {
488 - var markup = this.mountComponent(rootID, transaction);
490 + var markup = this.mountComponent(rootID, transaction, 0);
491 ReactComponent.mountImageIntoNode(markup, container, shouldReuseMarkup);
492 },
493
src/core/ReactCompositeComponent.js
+18 -4
@@ -531,6 +531,7 @@ var ReactCompositeComponentMixin = {
531 *
532 * @param {string} rootID DOM ID of the root node.
533 * @param {ReactReconcileTransaction} transaction
534 + * @param {number} mountDepth number of components in the owner hierarchy
535 * @return {?string} Rendered markup to be inserted into the DOM.
536 * @final
537 * @internal
@@ -538,8 +539,13 @@ var ReactCompositeComponentMixin = {
539 mountComponent: ReactPerf.measure(
540 'ReactCompositeComponent',
541 'mountComponent',
541 - function(rootID, transaction) {
542 - ReactComponent.Mixin.mountComponent.call(this, rootID, transaction);
542 + function(rootID, transaction, mountDepth) {
543 + ReactComponent.Mixin.mountComponent.call(
544 + this,
545 + rootID,
546 + transaction,
547 + mountDepth
548 + );
549 this._compositeLifeCycleState = CompositeLifeCycle.MOUNTING;
550
551 this._defaultProps = this.getDefaultProps ? this.getDefaultProps() : null;
@@ -567,7 +573,11 @@ var ReactCompositeComponentMixin = {
573
574 // Done with mounting, `setState` will now trigger UI changes.
575 this._compositeLifeCycleState = null;
570 - var markup = this._renderedComponent.mountComponent(rootID, transaction);
576 + var markup = this._renderedComponent.mountComponent(
577 + rootID,
578 + transaction,
579 + mountDepth + 1
580 + );
581 if (this.componentDidMount) {
582 transaction.getReactOnDOMReady().enqueue(this, this.componentDidMount);
583 }
@@ -788,7 +798,11 @@ var ReactCompositeComponentMixin = {
798 var thisID = this._rootNodeID;
799 var currentComponentID = currentComponent._rootNodeID;
800 currentComponent.unmountComponent();
791 - var nextMarkup = nextComponent.mountComponent(thisID, transaction);
801 + var nextMarkup = nextComponent.mountComponent(
802 + thisID,
803 + transaction,
804 + this._mountDepth + 1
805 + );
806 ReactComponent.DOMIDOperations.dangerouslyReplaceNodeWithMarkupByID(
807 currentComponentID,
808 nextMarkup
src/core/ReactMultiChild.js
+10 -2
@@ -202,7 +202,11 @@ var ReactMultiChild = {
202 if (children.hasOwnProperty(name) && child) {
203 // Inlined for performance, see `ReactInstanceHandles.createReactID`.
204 var rootID = this._rootNodeID + '.' + name;
205 - var mountImage = child.mountComponent(rootID, transaction);
205 + var mountImage = child.mountComponent(
206 + rootID,
207 + transaction,
208 + this._mountDepth + 1
209 + );
210 child._mountImage = mountImage;
211 child._mountIndex = index;
212 mountImages.push(mountImage);
@@ -395,7 +399,11 @@ var ReactMultiChild = {
399 _mountChildByNameAtIndex: function(child, name, index, transaction) {
400 // Inlined for performance, see `ReactInstanceHandles.createReactID`.
401 var rootID = this._rootNodeID + '.' + name;
398 - var mountImage = child.mountComponent(rootID, transaction);
402 + var mountImage = child.mountComponent(
403 + rootID,
404 + transaction,
405 + this._mountDepth + 1
406 + );
407 child._mountImage = mountImage;
408 child._mountIndex = index;
409 this.createChild(child);
src/core/ReactNativeComponent.js
+8 -2
@@ -84,13 +84,19 @@ ReactNativeComponent.Mixin = {
84 * @internal
85 * @param {string} rootID The root DOM ID for this node.
86 * @param {ReactReconcileTransaction} transaction
87 + * @param {number} mountDepth number of components in the owner hierarchy
88 * @return {string} The computed markup.
89 */
90 mountComponent: ReactPerf.measure(
91 'ReactNativeComponent',
92 'mountComponent',
92 - function(rootID, transaction) {
93 - ReactComponent.Mixin.mountComponent.call(this, rootID, transaction);
93 + function(rootID, transaction, mountDepth) {
94 + ReactComponent.Mixin.mountComponent.call(
95 + this,
96 + rootID,
97 + transaction,
98 + mountDepth
99 + );
100 assertValidProps(this.props);
101 return (
102 this._createOpenTagMarkup() +
src/core/ReactTextComponent.js
+9 -2
@@ -52,11 +52,18 @@ mixInto(ReactTextComponent, {
52 * any features besides containing text content.
53 *
54 * @param {string} rootID DOM ID of the root node.
55 + * @param {ReactReconcileTransaction} transaction
56 + * @param {number} mountDepth number of components in the owner hierarchy
57 * @return {string} Markup for this text node.
58 * @internal
59 */
58 - mountComponent: function(rootID) {
59 - ReactComponent.Mixin.mountComponent.call(this, rootID);
60 + mountComponent: function(rootID, transaction, mountDepth) {
61 + ReactComponent.Mixin.mountComponent.call(
62 + this,
63 + rootID,
64 + transaction,
65 + mountDepth
66 + );
67 return (
68 '<span ' + ReactMount.ATTR_NAME + '="' + rootID + '">' +
69 escapeTextForBrowser(this.props.text) +
src/core/ReactUpdates.js
+17 -1
@@ -33,8 +33,24 @@ function batchedUpdates(callback, param) {
33 batchingStrategy.batchedUpdates(callback, param);
34 }
35
36 +/**
37 + * Array comparator for ReactComponents by owner depth
38 + *
39 + * @param {ReactComponent} c1 first component you're comparing
40 + * @param {ReactComponent} c2 second component you're comparing
41 + * @return {number} Return value usable by Array.prototype.sort().
42 + */
43 +function mountDepthComparator(c1, c2) {
44 + return c1._mountDepth - c2._mountDepth;
45 +}
46 +
47 function runBatchedUpdates() {
37 - // TODO: Sort components by depth such that parent components update first
48 + // Since reconciling a component higher in the owner hierarchy usually (not
49 + // always -- see shouldComponentUpdate()) will reconcile children, reconcile
50 + // them before their children by sorting the array.
51 +
52 + dirtyComponents.sort(mountDepthComparator);
53 +
54 for (var i = 0; i < dirtyComponents.length; i++) {
55 // If a component is unmounted before pending changes apply, ignore them
56 // TODO: Queue unmounts in the same list to avoid this happening at all
src/core/__tests__/ReactComponent-test.js
+75
@@ -133,4 +133,79 @@ describe('ReactComponent', function() {
133 expect(instance.isMounted()).toBeTruthy();
134 });
135
136 + it('should know its simple mount depth', function() {
137 + var Owner = React.createClass({
138 + render: function() {
139 + return <Child ref="child" />;
140 + }
141 + });
142 +
143 + var Child = React.createClass({
144 + render: function() {
145 + return <div />;
146 + }
147 + });
148 +
149 + var instance = <Owner />;
150 + ReactTestUtils.renderIntoDocument(instance);
151 + expect(instance._mountDepth).toBe(0);
152 + expect(instance.refs.child._mountDepth).toBe(1);
153 + });
154 +
155 + it('should know its (complicated) mount depth', function() {
156 + var Box = React.createClass({
157 + render: function() {
158 + return <div ref="boxDiv">{this.props.children}</div>;
159 + }
160 + });
161 +
162 + var Child = React.createClass({
163 + render: function() {
164 + return <span ref="span">child</span>;
165 + }
166 + });
167 +
168 + var Switcher = React.createClass({
169 + getInitialState: function() {
170 + return {tabKey: 'hello'};
171 + },
172 +
173 + render: function() {
174 + var child = this.props.children;
175 +
176 + return (
177 + <Box ref="box">
178 + <div
179 + ref="switcherDiv"
180 + style={{
181 + display: this.state.tabKey === child.key ? '' : 'none'
182 + }}>
183 + {child}
184 + </div>
185 + </Box>
186 + );
187 + }
188 + });
189 +
190 + var App = React.createClass({
191 + render: function() {
192 + return (
193 + <Switcher ref="switcher">
194 + <Child key="hello" ref="child" />
195 + </Switcher>
196 + );
197 + }
198 + });
199 +
200 + var root = <App />;
201 + ReactTestUtils.renderIntoDocument(root);
202 +
203 + expect(root._mountDepth).toBe(0);
204 + expect(root.refs.switcher._mountDepth).toBe(1);
205 + expect(root.refs.switcher.refs.box._mountDepth).toBe(2);
206 + expect(root.refs.switcher.refs.switcherDiv._mountDepth).toBe(4);
207 + expect(root.refs.child._mountDepth).toBe(5);
208 + expect(root.refs.switcher.refs.box.refs.boxDiv._mountDepth).toBe(3);
209 + expect(root.refs.child.refs.span._mountDepth).toBe(6);
210 + });
211 });
src/core/__tests__/ReactNativeComponent-test.js
+1 -1
@@ -290,7 +290,7 @@ describe('ReactNativeComponent', function() {
290 mountComponent = function(props) {
291 var transaction = new ReactReconcileTransaction();
292 var stubComponent = new StubNativeComponent(props);
293 - return stubComponent.mountComponent('test', transaction);
293 + return stubComponent.mountComponent('test', transaction, 0);
294 };
295 });
296
src/core/__tests__/ReactUpdates-test.js
+167 -4
@@ -210,10 +210,8 @@ describe('ReactUpdates', function() {
210 expect(child.state.y).toBe(2);
211 expect(parentUpdateCount).toBe(1);
212
213 - // When we update the child first, we currently incur two updates because
214 - // we aren't smart about what order to process the components in.
215 - // TODO: Reduce the update count here to 1
216 - expect(childUpdateCount).toBe(2);
213 + // Batching reduces the number of updates here to 1.
214 + expect(childUpdateCount).toBe(1);
215 });
216
217 it('should support chained state updates', function() {
@@ -293,4 +291,169 @@ describe('ReactUpdates', function() {
291 expect(instance.state.x).toBe(1);
292 expect(updateCount).toBe(1);
293 });
294 +
295 + it('should update children even if parent blocks updates', function() {
296 + var parentRenderCount = 0;
297 + var childRenderCount = 0;
298 +
299 + var Parent = React.createClass({
300 + shouldComponentUpdate: function() {
301 + return false;
302 + },
303 +
304 + render: function() {
305 + parentRenderCount++;
306 + return <Child ref="child" />;
307 + }
308 + });
309 +
310 + var Child = React.createClass({
311 + render: function() {
312 + childRenderCount++;
313 + return <div />;
314 + }
315 + });
316 +
317 + expect(parentRenderCount).toBe(0);
318 + expect(childRenderCount).toBe(0);
319 +
320 + var instance = <Parent />;
321 + ReactTestUtils.renderIntoDocument(instance);
322 +
323 + expect(parentRenderCount).toBe(1);
324 + expect(childRenderCount).toBe(1);
325 +
326 + ReactUpdates.batchedUpdates(function() {
327 + instance.setState({x: 1});
328 + });
329 +
330 + expect(parentRenderCount).toBe(1);
331 + expect(childRenderCount).toBe(1);
332 +
333 + ReactUpdates.batchedUpdates(function() {
334 + instance.refs.child.setState({x: 1});
335 + });
336 +
337 + expect(parentRenderCount).toBe(1);
338 + expect(childRenderCount).toBe(2);
339 + });
340 +
341 + it('should flow updates correctly', function() {
342 + var willUpdates = [];
343 + var didUpdates = [];
344 +
345 + var UpdateLoggingMixin = {
346 + componentWillUpdate: function() {
347 + willUpdates.push(this.constructor.displayName);
348 + },
349 + componentDidUpdate: function() {
350 + didUpdates.push(this.constructor.displayName);
351 + }
352 + };
353 +
354 + var Box = React.createClass({
355 + mixins: [UpdateLoggingMixin],
356 +
357 + render: function() {
358 + return <div ref="boxDiv">{this.props.children}</div>;
359 + }
360 + });
361 +
362 + var Child = React.createClass({
363 + mixins: [UpdateLoggingMixin],
364 +
365 + render: function() {
366 + return <span ref="span">child</span>;
367 + }
368 + });
369 +
370 + var Switcher = React.createClass({
371 + mixins: [UpdateLoggingMixin],
372 +
373 + getInitialState: function() {
374 + return {tabKey: 'hello'};
375 + },
376 +
377 + render: function() {
378 + var child = this.props.children;
379 +
380 + return (
381 + <Box ref="box">
382 + <div
383 + ref="switcherDiv"
384 + style={{
385 + display: this.state.tabKey === child.key ? '' : 'none'
386 + }}>
387 + {child}
388 + </div>
389 + </Box>
390 + );
391 + }
392 + });
393 +
394 + var App = React.createClass({
395 + mixins: [UpdateLoggingMixin],
396 +
397 + render: function() {
398 + return (
399 + <Switcher ref="switcher">
400 + <Child key="hello" ref="child" />
401 + </Switcher>
402 + );
403 + }
404 + });
405 +
406 + var root = <App />;
407 + ReactTestUtils.renderIntoDocument(root);
408 +
409 + function expectUpdates(sequence) {
410 + // didUpdate() occurs in reverse order
411 + didUpdates.reverse();
412 + expect(willUpdates).toEqual(didUpdates);
413 + expect(willUpdates).toEqual(sequence);
414 + willUpdates.length = 0;
415 + didUpdates.length = 0;
416 + }
417 +
418 + function triggerUpdate(c) {
419 + c.setState({x: 1});
420 + }
421 +
422 + function testUpdates(components, expectation) {
423 + var i;
424 +
425 + ReactUpdates.batchedUpdates(function() {
426 + for (i = 0; i < components.length; i++) {
427 + triggerUpdate(components[i]);
428 + }
429 + });
430 +
431 + expectUpdates(expectation);
432 +
433 + // Try them in reverse order
434 +
435 + ReactUpdates.batchedUpdates(function() {
436 + for (i = components.length - 1; i >= 0; i--) {
437 + triggerUpdate(components[i]);
438 + }
439 + });
440 +
441 + expectUpdates(expectation);
442 + }
443 +
444 + testUpdates(
445 + [root.refs.switcher.refs.box, root.refs.switcher],
446 + ['Switcher', 'Box', 'Child']
447 + );
448 +
449 + testUpdates(
450 + [root.refs.child, root.refs.switcher.refs.box],
451 + ['Box', 'Child']
452 + );
453 +
454 + testUpdates(
455 + [root.refs.child, root.refs.switcher],
456 + ['Switcher', 'Box', 'Child']
457 + );
458 + });
459 });
src/dom/__tests__/Danger-test.js
+7 -3
@@ -36,14 +36,18 @@ describe('Danger', function() {
36 });
37
38 it('should render markup', function() {
39 - var markup = (<div />).mountComponent('.rX', transaction);
39 + var markup = (<div />).mountComponent('.rX', transaction, 0);
40 var output = Danger.dangerouslyRenderMarkup([markup])[0];
41
42 expect(output.nodeName).toBe('DIV');
43 });
44
45 it('should render markup with props', function() {
46 - var markup = (<div className="foo" />).mountComponent('.rX', transaction);
46 + var markup = (<div className="foo" />).mountComponent(
47 + '.rX',
48 + transaction,
49 + 0
50 + );
51 var output = Danger.dangerouslyRenderMarkup([markup])[0];
52
53 expect(output.nodeName).toBe('DIV');
@@ -51,7 +55,7 @@ describe('Danger', function() {
55 });
56
57 it('should render wrapped markup', function() {
54 - var markup = (<th />).mountComponent('.rX', transaction);
58 + var markup = (<th />).mountComponent('.rX', transaction, 0);
59 var output = Danger.dangerouslyRenderMarkup([markup])[0];
60
61 expect(output.nodeName).toBe('TH');
src/environment/ReactServerRendering.js
+1 -1
@@ -34,7 +34,7 @@ function renderComponentToString(component, callback) {
34 transaction.reinitializeTransaction();
35 try {
36 transaction.perform(function() {
37 - var markup = component.mountComponent(id, transaction);
37 + var markup = component.mountComponent(id, transaction, 0);
38 markup = ReactMarkupChecksum.addChecksumToMarkup(markup);
39 callback(markup);
40 }, null);