From ace9e8134c3080d86f20097d5ba3369e15a97a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 3 Feb 2020 12:47:14 -0800 Subject: [PATCH] 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. --- ...MServerSelectiveHydration-test.internal.js | 204 +++++++++++++++++- .../src/ReactFiberExpirationTime.js | 16 +- .../src/ReactFiberReconciler.js | 9 +- 3 files changed, 205 insertions(+), 24 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index ca94bc3bc1e0b..02d1dff1a9ec6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -14,6 +14,7 @@ import {createEventTarget} from 'dom-event-testing-library'; let React; let ReactDOM; let ReactDOMServer; +let ReactTestUtils; let Scheduler; let Suspense; let usePress; @@ -102,6 +103,7 @@ describe('ReactDOMServerSelectiveHydration', () => { React = require('react'); ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); + ReactTestUtils = require('react-dom/test-utils'); Scheduler = require('scheduler'); Suspense = React.Suspense; usePress = require('react-interactions/events/press').usePress; @@ -585,7 +587,7 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.removeChild(container); }); - it('hydrates the last target as higher priority for continuous events', async () => { + it('hydrates the hovered targets as higher priority for continuous events', async () => { let suspend = false; let resolve; let promise = new Promise(resolvePromise => (resolve = resolvePromise)); @@ -669,21 +671,107 @@ describe('ReactDOMServerSelectiveHydration', () => { // We should prioritize hydrating D first because we clicked it. // Next we should hydrate C since that's the current hover target. - // Next it doesn't matter if we hydrate A or B first but as an - // implementation detail we're currently hydrating B first since - // we at one point hovered over it and we never deprioritized it. + // To simplify implementation details we hydrate both B and C at + // the same time since B was already scheduled. + // This is ok because it will at least not continue for nested + // boundary. See the next test below. expect(Scheduler).toFlushAndYield([ 'D', 'Clicked D', + 'B', // Ideally this should be later. 'C', 'Hover C', - 'B', 'A', ]); document.body.removeChild(container); }); + it('hydrates the last target path first for continuous events', async () => { + let suspend = false; + let resolve; + let promise = new Promise(resolvePromise => (resolve = resolvePromise)); + + function Child({text}) { + if ((text === 'A' || text === 'D') && suspend) { + throw promise; + } + Scheduler.unstable_yieldValue(text); + return ( + { + e.preventDefault(); + Scheduler.unstable_yieldValue('Hover ' + text); + }}> + {text} + + ); + } + + function App() { + Scheduler.unstable_yieldValue('App'); + return ( +
+ + + + +
+ + + +
+ + + + + +
+ ); + } + + let finalHTML = ReactDOMServer.renderToString(); + + expect(Scheduler).toHaveYielded(['App', 'A', 'B', 'C', 'D']); + + let container = document.createElement('div'); + // We need this to be in the document since we'll dispatch events on it. + document.body.appendChild(container); + + container.innerHTML = finalHTML; + + let spanB = container.getElementsByTagName('span')[1]; + let spanC = container.getElementsByTagName('span')[2]; + let spanD = container.getElementsByTagName('span')[3]; + + suspend = true; + + // A and D will be suspended. We'll click on D which should take + // priority, after we unsuspend. + let root = ReactDOM.createRoot(container, {hydrate: true}); + root.render(); + + // Nothing has been hydrated so far. + expect(Scheduler).toHaveYielded([]); + + // Hover over B and then C. + dispatchMouseHoverEvent(spanB, spanD); + dispatchMouseHoverEvent(spanC, spanB); + + suspend = false; + resolve(); + await promise; + + // We should prioritize hydrating D first because we clicked it. + // Next we should hydrate C since that's the current hover target. + // Next it doesn't matter if we hydrate A or B first but as an + // implementation detail we're currently hydrating B first since + // we at one point hovered over it and we never deprioritized it. + expect(Scheduler).toFlushAndYield(['App', 'C', 'Hover C', 'A', 'B', 'D']); + + document.body.removeChild(container); + }); + it('hydrates the last explicitly hydrated target at higher priority', async () => { function Child({text}) { Scheduler.unstable_yieldValue(text); @@ -731,4 +819,110 @@ describe('ReactDOMServerSelectiveHydration', () => { // gets highest priority followed by the next added. expect(Scheduler).toFlushAndYield(['App', 'C', 'B', 'A']); }); + + it('hydrates before an update even if hydration moves away from it', async () => { + function Child({text}) { + Scheduler.unstable_yieldValue(text); + return {text}; + } + let ChildWithBoundary = React.memo(function({text}) { + return ( + + + + + ); + }); + + function App({a}) { + Scheduler.unstable_yieldValue('App'); + React.useEffect(() => { + Scheduler.unstable_yieldValue('Commit'); + }); + return ( +
+ + + +
+ ); + } + + let finalHTML = ReactDOMServer.renderToString(); + + expect(Scheduler).toHaveYielded(['App', 'A', 'a', 'B', 'b', 'C', 'c']); + + let container = document.createElement('div'); + container.innerHTML = finalHTML; + + // We need this to be in the document since we'll dispatch events on it. + document.body.appendChild(container); + + let spanA = container.getElementsByTagName('span')[0]; + let spanB = container.getElementsByTagName('span')[2]; + let spanC = container.getElementsByTagName('span')[4]; + + let root = ReactDOM.createRoot(container, {hydrate: true}); + ReactTestUtils.act(() => { + root.render(); + + // Hydrate the shell. + expect(Scheduler).toFlushAndYieldThrough(['App', 'Commit']); + + // Render an update at Idle priority that needs to update A. + Scheduler.unstable_runWithPriority( + Scheduler.unstable_IdlePriority, + () => { + root.render(); + }, + ); + + // Start rendering. This will force the first boundary to hydrate + // by scheduling it at one higher pri than Idle. + expect(Scheduler).toFlushAndYieldThrough(['App', 'A']); + + // Hover over A which (could) schedule at one higher pri than Idle. + dispatchMouseHoverEvent(spanA, null); + + // Before, we're done we now switch to hover over B. + // This is meant to test that this doesn't cause us to forget that + // we still have to hydrate A. The first boundary. + // This also tests that we don't do the -1 down-prioritization of + // continuous hover events because that would decrease its priority + // to Idle. + dispatchMouseHoverEvent(spanB, spanA); + + // Also click C to prioritize that even higher which resets the + // priority levels. + dispatchClickEvent(spanC); + + expect(Scheduler).toHaveYielded([ + // Hydrate C first since we clicked it. + 'C', + 'c', + ]); + + expect(Scheduler).toFlushAndYield([ + // Finish hydration of A since we forced it to hydrate. + 'A', + 'a', + // Also, hydrate B since we hovered over it. + // It's not important which one comes first. A or B. + // As long as they both happen before the Idle update. + 'B', + 'b', + // Begin the Idle update again. + 'App', + 'AA', + 'aa', + 'Commit', + ]); + }); + + let spanA2 = container.getElementsByTagName('span')[0]; + // This is supposed to have been hydrated, not replaced. + expect(spanA).toBe(spanA2); + + document.body.removeChild(container); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberExpirationTime.js b/packages/react-reconciler/src/ReactFiberExpirationTime.js index 5f4660dd5175f..72d58dff4eb9f 100644 --- a/packages/react-reconciler/src/ReactFiberExpirationTime.js +++ b/packages/react-reconciler/src/ReactFiberExpirationTime.js @@ -32,10 +32,9 @@ export const Never = 1; // Idle is slightly higher priority than Never. It must completely finish in // order to be consistent. export const Idle = 2; -// Continuous Hydration is a moving priority. It is slightly higher than Idle -// and is used to increase priority of hover targets. It is increasing with -// each usage so that last always wins. -let ContinuousHydration = 3; +// Continuous Hydration is slightly higher than Idle and is used to increase +// priority of hover targets. +export const ContinuousHydration = 3; export const Sync = MAX_SIGNED_31_BIT_INT; export const Batched = Sync - 1; @@ -119,15 +118,6 @@ export function computeInteractiveExpiration(currentTime: ExpirationTime) { ); } -export function computeContinuousHydrationExpiration( - currentTime: ExpirationTime, -) { - // Each time we ask for a new one of these we increase the priority. - // This ensures that the last one always wins since we can't deprioritize - // once we've scheduled work already. - return ContinuousHydration++; -} - export function inferPriorityFromExpirationTime( currentTime: ExpirationTime, expirationTime: ExpirationTime, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index fb46524645aee..8a91ea9b4c25c 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -76,8 +76,8 @@ import { import {StrictMode} from './ReactTypeOfMode'; import { Sync, + ContinuousHydration, computeInteractiveExpiration, - computeContinuousHydrationExpiration, } from './ReactFiberExpirationTime'; import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig'; import { @@ -384,11 +384,8 @@ export function attemptContinuousHydration(fiber: Fiber): void { // Suspense. return; } - let expTime = computeContinuousHydrationExpiration( - requestCurrentTimeForUpdate(), - ); - scheduleWork(fiber, expTime); - markRetryTimeIfNotHydrated(fiber, expTime); + scheduleWork(fiber, ContinuousHydration); + markRetryTimeIfNotHydrated(fiber, ContinuousHydration); } export function attemptHydrationAtCurrentPriority(fiber: Fiber): void {