@samitouri / QOS-React / commits / 4a924a2067

Updates at the same priority should not interrupt current render (#11578)

When we're rendering work at a specific level, and a higher priority update comes in, we interrupt the current work and restart at the higher priority. The rationale is that the high priority update is likely cheaper to render that the lower one, so it's usually worth throwing out the current work to get the high pri update on the screen as soon as possible. Currently, we also interrupt the current work if an update of *equal* priority is scheduled. The rationale here is less clear: the only reason to do this is if both updates are expected to flush at the same time, to prevent tearing. But this usually isn't the case. Separate setStates are usually distinct updates that can be flushed separately, especially if the components that are being updated are in separate subtrees. An exception is in Flux-like systems where multiple setStates are the result of a single conceptual update/event/dispatch. We can add an explicit API for batching in the future; in fact, we'd likely need one anyway to account for expiration accidentally causing consecutive updates to fall into separate buckets.

Andrew Clark committed Nov 16, 2017 at 16:59 UTC 4a924a206783df5d2e06755caa0fbd2dd580b14e
3 files changed +73 -2
packages/react-noop-renderer/src/ReactNoop.js
+5 -1
@@ -455,7 +455,11 @@ var ReactNoop = {
455
456 unbatchedUpdates: NoopRenderer.unbatchedUpdates,
457
458 - flushSync: NoopRenderer.flushSync,
458 + flushSync(fn: () => mixed) {
459 + yieldedValues = [];
460 + NoopRenderer.flushSync(fn);
461 + return yieldedValues;
462 + },
463
464 // Logs the current state of the tree.
465 dumpTree(rootID: string = DEFAULT_ROOT_ID) {
packages/react-reconciler/src/ReactFiberScheduler.js
+1 -1
@@ -1202,7 +1202,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
1202 if (
1203 !isWorking &&
1204 root === nextRoot &&
1205 - expirationTime <= nextRenderExpirationTime
1205 + expirationTime < nextRenderExpirationTime
1206 ) {
1207 // Restart the root from the top.
1208 if (nextUnitOfWork !== null) {
packages/react-reconciler/src/__tests__/ReactIncremental-test.js
+67
@@ -168,6 +168,7 @@ describe('ReactIncremental', () => {
168 ops = [];
169
170 // This will abort the previous work and restart
171 + ReactNoop.flushSync(() => ReactNoop.render(null));
172 ReactNoop.render(<Foo text="baz" />);
173
174 // Flush part of the new work
@@ -221,6 +222,7 @@ describe('ReactIncremental', () => {
222 expect(ops).toEqual(['setState1']);
223
224 // This will abort the previous work and restart
225 + ReactNoop.flushSync(() => ReactNoop.render(<Foo />));
226 inst.setState(
227 () => {
228 ops.push('setState2');
@@ -1877,6 +1879,8 @@ describe('ReactIncremental', () => {
1879 );
1880 ReactNoop.flush();
1881 expect(ops).toEqual([
1882 + 'ShowLocale {"locale":"sv"}',
1883 + 'ShowBoth {"locale":"sv"}',
1884 'Intl {}',
1885 'ShowLocale {"locale":"en"}',
1886 'Router {}',
@@ -2648,4 +2652,67 @@ describe('ReactIncremental', () => {
2652 'count:1, name:not brian',
2653 ]);
2654 });
2655 +
2656 + it('does not interrupt for update at same priority', () => {
2657 + function Parent(props) {
2658 + ReactNoop.yield('Parent: ' + props.step);
2659 + return <Child step={props.step} />;
2660 + }
2661 +
2662 + function Child(props) {
2663 + ReactNoop.yield('Child: ' + props.step);
2664 + return null;
2665 + }
2666 +
2667 + ReactNoop.render(<Parent step={1} />);
2668 + ReactNoop.flushThrough(['Parent: 1']);
2669 +
2670 + // Interrupt at same priority
2671 + ReactNoop.render(<Parent step={2} />);
2672 +
2673 + expect(ReactNoop.flush()).toEqual(['Child: 1', 'Parent: 2', 'Child: 2']);
2674 + });
2675 +
2676 + it('does not interrupt for update at lower priority', () => {
2677 + function Parent(props) {
2678 + ReactNoop.yield('Parent: ' + props.step);
2679 + return <Child step={props.step} />;
2680 + }
2681 +
2682 + function Child(props) {
2683 + ReactNoop.yield('Child: ' + props.step);
2684 + return null;
2685 + }
2686 +
2687 + ReactNoop.render(<Parent step={1} />);
2688 + ReactNoop.flushThrough(['Parent: 1']);
2689 +
2690 + // Interrupt at lower priority
2691 + ReactNoop.expire(2000);
2692 + ReactNoop.render(<Parent step={2} />);
2693 +
2694 + expect(ReactNoop.flush()).toEqual(['Child: 1', 'Parent: 2', 'Child: 2']);
2695 + });
2696 +
2697 + it('does interrupt for update at higher priority', () => {
2698 + function Parent(props) {
2699 + ReactNoop.yield('Parent: ' + props.step);
2700 + return <Child step={props.step} />;
2701 + }
2702 +
2703 + function Child(props) {
2704 + ReactNoop.yield('Child: ' + props.step);
2705 + return null;
2706 + }
2707 +
2708 + ReactNoop.render(<Parent step={1} />);
2709 + ReactNoop.flushThrough(['Parent: 1']);
2710 +
2711 + // Interrupt at higher priority
2712 + expect(
2713 + ReactNoop.flushSync(() => ReactNoop.render(<Parent step={2} />)),
2714 + ).toEqual(['Parent: 2', 'Child: 2']);
2715 +
2716 + expect(ReactNoop.flush()).toEqual([]);
2717 + });
2718 });