@samitouri / QOS-React-2 / commits / ace9e8134c

Simplify Continuous Hydration Targets (#17952)

* Simplify Continuous Hydration Targets Let's use a constant priority for this. This helps us avoid restarting a render when switching targets and simplifies the model. The downside is that now we're not down-prioritizing the previous hover target. However, we think that's ok because it'll only do one level too much and then stop. * Add test meant to show why it's tricky to merge both hydration levels Having both levels co-exist works. However, if we deprioritize hydration using a single level, we might deprioritize the wrong thing. This adds a test that catches it if we ever try a naive deprioritization in the future. It also tests that we don't down-prioritize if we're changing the hover in the middle of doing continuous priority work.

Sebastian Markbåge committed Feb 3, 2020 at 12:47 UTC ace9e8134c3080d86f20097d5ba3369e15a97a83
3 files changed +205 -24
packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js
+199 -5
@@ -14,6 +14,7 @@ import {createEventTarget} from 'dom-event-testing-library';
14 let React;
15 let ReactDOM;
16 let ReactDOMServer;
17 +let ReactTestUtils;
18 let Scheduler;
19 let Suspense;
20 let usePress;
@@ -102,6 +103,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
103 React = require('react');
104 ReactDOM = require('react-dom');
105 ReactDOMServer = require('react-dom/server');
106 + ReactTestUtils = require('react-dom/test-utils');
107 Scheduler = require('scheduler');
108 Suspense = React.Suspense;
109 usePress = require('react-interactions/events/press').usePress;
@@ -585,7 +587,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
587 document.body.removeChild(container);
588 });
589
588 - it('hydrates the last target as higher priority for continuous events', async () => {
590 + it('hydrates the hovered targets as higher priority for continuous events', async () => {
591 let suspend = false;
592 let resolve;
593 let promise = new Promise(resolvePromise => (resolve = resolvePromise));
@@ -669,21 +671,107 @@ describe('ReactDOMServerSelectiveHydration', () => {
671
672 // We should prioritize hydrating D first because we clicked it.
673 // Next we should hydrate C since that's the current hover target.
672 - // Next it doesn't matter if we hydrate A or B first but as an
673 - // implementation detail we're currently hydrating B first since
674 - // we at one point hovered over it and we never deprioritized it.
674 + // To simplify implementation details we hydrate both B and C at
675 + // the same time since B was already scheduled.
676 + // This is ok because it will at least not continue for nested
677 + // boundary. See the next test below.
678 expect(Scheduler).toFlushAndYield([
679 'D',
680 'Clicked D',
681 + 'B', // Ideally this should be later.
682 'C',
683 'Hover C',
680 - 'B',
684 'A',
685 ]);
686
687 document.body.removeChild(container);
688 });
689
690 + it('hydrates the last target path first for continuous events', async () => {
691 + let suspend = false;
692 + let resolve;
693 + let promise = new Promise(resolvePromise => (resolve = resolvePromise));
694 +
695 + function Child({text}) {
696 + if ((text === 'A' || text === 'D') && suspend) {
697 + throw promise;
698 + }
699 + Scheduler.unstable_yieldValue(text);
700 + return (
701 + <span
702 + onMouseEnter={e => {
703 + e.preventDefault();
704 + Scheduler.unstable_yieldValue('Hover ' + text);
705 + }}>
706 + {text}
707 + </span>
708 + );
709 + }
710 +
711 + function App() {
712 + Scheduler.unstable_yieldValue('App');
713 + return (
714 + <div>
715 + <Suspense fallback="Loading...">
716 + <Child text="A" />
717 + </Suspense>
718 + <Suspense fallback="Loading...">
719 + <div>
720 + <Suspense fallback="Loading...">
721 + <Child text="B" />
722 + </Suspense>
723 + </div>
724 + <Child text="C" />
725 + </Suspense>
726 + <Suspense fallback="Loading...">
727 + <Child text="D" />
728 + </Suspense>
729 + </div>
730 + );
731 + }
732 +
733 + let finalHTML = ReactDOMServer.renderToString(<App />);
734 +
735 + expect(Scheduler).toHaveYielded(['App', 'A', 'B', 'C', 'D']);
736 +
737 + let container = document.createElement('div');
738 + // We need this to be in the document since we'll dispatch events on it.
739 + document.body.appendChild(container);
740 +
741 + container.innerHTML = finalHTML;
742 +
743 + let spanB = container.getElementsByTagName('span')[1];
744 + let spanC = container.getElementsByTagName('span')[2];
745 + let spanD = container.getElementsByTagName('span')[3];
746 +
747 + suspend = true;
748 +
749 + // A and D will be suspended. We'll click on D which should take
750 + // priority, after we unsuspend.
751 + let root = ReactDOM.createRoot(container, {hydrate: true});
752 + root.render(<App />);
753 +
754 + // Nothing has been hydrated so far.
755 + expect(Scheduler).toHaveYielded([]);
756 +
757 + // Hover over B and then C.
758 + dispatchMouseHoverEvent(spanB, spanD);
759 + dispatchMouseHoverEvent(spanC, spanB);
760 +
761 + suspend = false;
762 + resolve();
763 + await promise;
764 +
765 + // We should prioritize hydrating D first because we clicked it.
766 + // Next we should hydrate C since that's the current hover target.
767 + // Next it doesn't matter if we hydrate A or B first but as an
768 + // implementation detail we're currently hydrating B first since
769 + // we at one point hovered over it and we never deprioritized it.
770 + expect(Scheduler).toFlushAndYield(['App', 'C', 'Hover C', 'A', 'B', 'D']);
771 +
772 + document.body.removeChild(container);
773 + });
774 +
775 it('hydrates the last explicitly hydrated target at higher priority', async () => {
776 function Child({text}) {
777 Scheduler.unstable_yieldValue(text);
@@ -731,4 +819,110 @@ describe('ReactDOMServerSelectiveHydration', () => {
819 // gets highest priority followed by the next added.
820 expect(Scheduler).toFlushAndYield(['App', 'C', 'B', 'A']);
821 });
822 +
823 + it('hydrates before an update even if hydration moves away from it', async () => {
824 + function Child({text}) {
825 + Scheduler.unstable_yieldValue(text);
826 + return <span>{text}</span>;
827 + }
828 + let ChildWithBoundary = React.memo(function({text}) {
829 + return (
830 + <Suspense fallback="Loading...">
831 + <Child text={text} />
832 + <Child text={text.toLowerCase()} />
833 + </Suspense>
834 + );
835 + });
836 +
837 + function App({a}) {
838 + Scheduler.unstable_yieldValue('App');
839 + React.useEffect(() => {
840 + Scheduler.unstable_yieldValue('Commit');
841 + });
842 + return (
843 + <div>
844 + <ChildWithBoundary text={a} />
845 + <ChildWithBoundary text="B" />
846 + <ChildWithBoundary text="C" />
847 + </div>
848 + );
849 + }
850 +
851 + let finalHTML = ReactDOMServer.renderToString(<App a="A" />);
852 +
853 + expect(Scheduler).toHaveYielded(['App', 'A', 'a', 'B', 'b', 'C', 'c']);
854 +
855 + let container = document.createElement('div');
856 + container.innerHTML = finalHTML;
857 +
858 + // We need this to be in the document since we'll dispatch events on it.
859 + document.body.appendChild(container);
860 +
861 + let spanA = container.getElementsByTagName('span')[0];
862 + let spanB = container.getElementsByTagName('span')[2];
863 + let spanC = container.getElementsByTagName('span')[4];
864 +
865 + let root = ReactDOM.createRoot(container, {hydrate: true});
866 + ReactTestUtils.act(() => {
867 + root.render(<App a="A" />);
868 +
869 + // Hydrate the shell.
870 + expect(Scheduler).toFlushAndYieldThrough(['App', 'Commit']);
871 +
872 + // Render an update at Idle priority that needs to update A.
873 + Scheduler.unstable_runWithPriority(
874 + Scheduler.unstable_IdlePriority,
875 + () => {
876 + root.render(<App a="AA" />);
877 + },
878 + );
879 +
880 + // Start rendering. This will force the first boundary to hydrate
881 + // by scheduling it at one higher pri than Idle.
882 + expect(Scheduler).toFlushAndYieldThrough(['App', 'A']);
883 +
884 + // Hover over A which (could) schedule at one higher pri than Idle.
885 + dispatchMouseHoverEvent(spanA, null);
886 +
887 + // Before, we're done we now switch to hover over B.
888 + // This is meant to test that this doesn't cause us to forget that
889 + // we still have to hydrate A. The first boundary.
890 + // This also tests that we don't do the -1 down-prioritization of
891 + // continuous hover events because that would decrease its priority
892 + // to Idle.
893 + dispatchMouseHoverEvent(spanB, spanA);
894 +
895 + // Also click C to prioritize that even higher which resets the
896 + // priority levels.
897 + dispatchClickEvent(spanC);
898 +
899 + expect(Scheduler).toHaveYielded([
900 + // Hydrate C first since we clicked it.
901 + 'C',
902 + 'c',
903 + ]);
904 +
905 + expect(Scheduler).toFlushAndYield([
906 + // Finish hydration of A since we forced it to hydrate.
907 + 'A',
908 + 'a',
909 + // Also, hydrate B since we hovered over it.
910 + // It's not important which one comes first. A or B.
911 + // As long as they both happen before the Idle update.
912 + 'B',
913 + 'b',
914 + // Begin the Idle update again.
915 + 'App',
916 + 'AA',
917 + 'aa',
918 + 'Commit',
919 + ]);
920 + });
921 +
922 + let spanA2 = container.getElementsByTagName('span')[0];
923 + // This is supposed to have been hydrated, not replaced.
924 + expect(spanA).toBe(spanA2);
925 +
926 + document.body.removeChild(container);
927 + });
928 });
packages/react-reconciler/src/ReactFiberExpirationTime.js
+3 -13
@@ -32,10 +32,9 @@ export const Never = 1;
32 // Idle is slightly higher priority than Never. It must completely finish in
33 // order to be consistent.
34 export const Idle = 2;
35 -// Continuous Hydration is a moving priority. It is slightly higher than Idle
36 -// and is used to increase priority of hover targets. It is increasing with
37 -// each usage so that last always wins.
38 -let ContinuousHydration = 3;
35 +// Continuous Hydration is slightly higher than Idle and is used to increase
36 +// priority of hover targets.
37 +export const ContinuousHydration = 3;
38 export const Sync = MAX_SIGNED_31_BIT_INT;
39 export const Batched = Sync - 1;
40
@@ -119,15 +118,6 @@ export function computeInteractiveExpiration(currentTime: ExpirationTime) {
118 );
119 }
120
122 -export function computeContinuousHydrationExpiration(
123 - currentTime: ExpirationTime,
124 -) {
125 - // Each time we ask for a new one of these we increase the priority.
126 - // This ensures that the last one always wins since we can't deprioritize
127 - // once we've scheduled work already.
128 - return ContinuousHydration++;
129 -}
130 -
121 export function inferPriorityFromExpirationTime(
122 currentTime: ExpirationTime,
123 expirationTime: ExpirationTime,
packages/react-reconciler/src/ReactFiberReconciler.js
+3 -6
@@ -76,8 +76,8 @@ import {
76 import {StrictMode} from './ReactTypeOfMode';
77 import {
78 Sync,
79 + ContinuousHydration,
80 computeInteractiveExpiration,
80 - computeContinuousHydrationExpiration,
81 } from './ReactFiberExpirationTime';
82 import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
83 import {
@@ -384,11 +384,8 @@ export function attemptContinuousHydration(fiber: Fiber): void {
384 // Suspense.
385 return;
386 }
387 - let expTime = computeContinuousHydrationExpiration(
388 - requestCurrentTimeForUpdate(),
389 - );
390 - scheduleWork(fiber, expTime);
391 - markRetryTimeIfNotHydrated(fiber, expTime);
387 + scheduleWork(fiber, ContinuousHydration);
388 + markRetryTimeIfNotHydrated(fiber, ContinuousHydration);
389 }
390
391 export function attemptHydrationAtCurrentPriority(fiber: Fiber): void {