From 29ed08e54a4597730ca103c13590ffb9fc1bed2e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 26 Sep 2019 12:57:55 -0700 Subject: [PATCH 1/2] Increase retryTime for increased priority dehydrated boundaries --- ...MServerSelectiveHydration-test.internal.js | 84 +++++++++++++++++++ packages/react-dom/src/client/ReactDOM.js | 7 +- .../src/events/ReactDOMEventReplaying.js | 6 ++ .../src/ReactFiberReconciler.js | 38 ++++++++- 4 files changed, 133 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 06fa2de29886..120dfb5f7f06 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -114,4 +114,88 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.removeChild(container); }); + + it('hydrates at higher pri if sync did not work first time', 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('Clicked ' + 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 span = 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.unstable_createRoot(container, {hydrate: true}); + root.render(); + + // Nothing has been hydrated so far. + expect(Scheduler).toHaveYielded([]); + + // This click target cannot be hydrated yet because it's suspended. + let result = dispatchClickEvent(span); + + expect(Scheduler).toHaveYielded(['App']); + + expect(result).toBe(true); + + // Continuing rendering will render B next. + expect(Scheduler).toFlushAndYield(['B', 'C']); + + suspend = false; + resolve(); + await promise; + + // After the click, we should prioritize D and the Click first, + // and only after that render A and C. + expect(Scheduler).toFlushAndYield(['D', 'Clicked D', 'A']); + + document.body.removeChild(container); + }); }); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 7a7b4de04a5c..62f0ddfdef9c 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -40,6 +40,7 @@ import { flushPassiveEffects, IsThisRendererActing, attemptSynchronousHydration, + attemptHydration, } from 'react-reconciler/inline.dom'; import {createPortal as createPortalImpl} from 'shared/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -75,7 +76,10 @@ import { } from './ReactDOMComponentTree'; import {restoreControlledState} from './ReactDOMComponent'; import {dispatchEvent} from '../events/ReactDOMEventListener'; -import {setAttemptSynchronousHydration} from '../events/ReactDOMEventReplaying'; +import { + setAttemptSynchronousHydration, + setAttemptHydration, +} from '../events/ReactDOMEventReplaying'; import {eagerlyTrapReplayableEvents} from '../events/ReactDOMEventReplaying'; import { ELEMENT_NODE, @@ -86,6 +90,7 @@ import { import {ROOT_ATTRIBUTE_NAME} from '../shared/DOMProperty'; setAttemptSynchronousHydration(attemptSynchronousHydration); +setAttemptHydration(attemptHydration); const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; diff --git a/packages/react-dom/src/events/ReactDOMEventReplaying.js b/packages/react-dom/src/events/ReactDOMEventReplaying.js index 76ce3c715823..9edf0b847f3f 100644 --- a/packages/react-dom/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom/src/events/ReactDOMEventReplaying.js @@ -37,6 +37,12 @@ export function setAttemptSynchronousHydration(fn: (fiber: Object) => void) { attemptSynchronousHydration = fn; } +let attemptHydration: (fiber: Object) => void; + +export function setAttemptHydration(fn: (fiber: Object) => void) { + attemptHydration = fn; +} + // TODO: Upgrade this definition once we're on a newer version of Flow that // has this definition built-in. type PointerEvent = Event & { diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 4aa0219ca9bc..1a04a1ffe5a0 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -20,7 +20,10 @@ import {FundamentalComponent} from 'shared/ReactWorkTags'; import type {ReactNodeList} from 'shared/ReactTypes'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; -import type {SuspenseHydrationCallbacks} from './ReactFiberSuspenseComponent'; +import type { + SuspenseHydrationCallbacks, + SuspenseState, +} from './ReactFiberSuspenseComponent'; import { findCurrentHostFiber, @@ -378,10 +381,43 @@ export function attemptSynchronousHydration(fiber: Fiber): void { break; case SuspenseComponent: flushSync(() => scheduleWork(fiber, Sync)); + // If we're still blocked after this, we need to increase + // the priority of any promises resolving within this + // boundary so that they next attempt also has higher pri. + markRetryTimeIfNotHydrated(fiber, Sync); break; } } +function markRetryTimeImpl(fiber: Fiber, retryTime: ExpirationTime) { + let suspenseState: null | SuspenseState = fiber.memoizedState; + if (suspenseState !== null && suspenseState.dehydrated !== null) { + if (suspenseState.retryTime < retryTime) { + suspenseState.retryTime = retryTime; + } + } +} + +// Increases the priority of thennables when they resolve within this boundary. +function markRetryTimeIfNotHydrated(fiber: Fiber, retryTime: ExpirationTime) { + markRetryTimeImpl(fiber, retryTime); + let alternate = fiber.alternate; + if (alternate) { + markRetryTimeImpl(alternate, retryTime); + } +} + +export function attemptHydration(fiber: Fiber): void { + if (fiber.tag !== SuspenseComponent) { + // We ignore HostRoots here because we can't increase + // their priority and they should not suspend on I/O, + // since you have to wrap anything that might suspend in + // Suspense. + return; + } + // TODO +} + export {findHostInstance}; export {findHostInstanceWithWarning}; From 3381578648c2e3a73abca6e9fab05771a5d776d0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 27 Sep 2019 16:29:58 -0700 Subject: [PATCH 2/2] Increaese the priority to user blocking for every next discrete boundary --- ...MServerSelectiveHydration-test.internal.js | 96 ++++++++++++++++++- packages/react-dom/src/client/ReactDOM.js | 6 +- .../src/events/ReactDOMEventReplaying.js | 12 ++- .../src/ReactFiberReconciler.js | 11 ++- 4 files changed, 113 insertions(+), 12 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 120dfb5f7f06..72942d4f2dd6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -166,7 +166,7 @@ describe('ReactDOMServerSelectiveHydration', () => { container.innerHTML = finalHTML; - let span = container.getElementsByTagName('span')[3]; + let spanD = container.getElementsByTagName('span')[3]; suspend = true; @@ -179,7 +179,7 @@ describe('ReactDOMServerSelectiveHydration', () => { expect(Scheduler).toHaveYielded([]); // This click target cannot be hydrated yet because it's suspended. - let result = dispatchClickEvent(span); + let result = dispatchClickEvent(spanD); expect(Scheduler).toHaveYielded(['App']); @@ -198,4 +198,96 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.removeChild(container); }); + + it('hydrates at higher pri for secondary discrete 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('Clicked ' + 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 spanA = container.getElementsByTagName('span')[0]; + 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.unstable_createRoot(container, {hydrate: true}); + root.render(); + + // Nothing has been hydrated so far. + expect(Scheduler).toHaveYielded([]); + + // This click target cannot be hydrated yet because the first is Suspended. + dispatchClickEvent(spanA); + dispatchClickEvent(spanC); + dispatchClickEvent(spanD); + + expect(Scheduler).toHaveYielded(['App']); + + suspend = false; + resolve(); + await promise; + + // We should prioritize hydrating A, C and D first since we clicked in + // them. Only after they're done will we hydrate B. + expect(Scheduler).toFlushAndYield([ + 'A', + 'Clicked A', + 'C', + 'Clicked C', + 'D', + 'Clicked D', + // B should render last since it wasn't clicked. + 'B', + ]); + + document.body.removeChild(container); + }); }); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 62f0ddfdef9c..27b0eb88f1f9 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -40,7 +40,7 @@ import { flushPassiveEffects, IsThisRendererActing, attemptSynchronousHydration, - attemptHydration, + attemptUserBlockingHydration, } from 'react-reconciler/inline.dom'; import {createPortal as createPortalImpl} from 'shared/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -78,7 +78,7 @@ import {restoreControlledState} from './ReactDOMComponent'; import {dispatchEvent} from '../events/ReactDOMEventListener'; import { setAttemptSynchronousHydration, - setAttemptHydration, + setAttemptUserBlockingHydration, } from '../events/ReactDOMEventReplaying'; import {eagerlyTrapReplayableEvents} from '../events/ReactDOMEventReplaying'; import { @@ -90,7 +90,7 @@ import { import {ROOT_ATTRIBUTE_NAME} from '../shared/DOMProperty'; setAttemptSynchronousHydration(attemptSynchronousHydration); -setAttemptHydration(attemptHydration); +setAttemptUserBlockingHydration(attemptUserBlockingHydration); const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; diff --git a/packages/react-dom/src/events/ReactDOMEventReplaying.js b/packages/react-dom/src/events/ReactDOMEventReplaying.js index 9edf0b847f3f..866b8a492e33 100644 --- a/packages/react-dom/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom/src/events/ReactDOMEventReplaying.js @@ -37,10 +37,10 @@ export function setAttemptSynchronousHydration(fn: (fiber: Object) => void) { attemptSynchronousHydration = fn; } -let attemptHydration: (fiber: Object) => void; +let attemptUserBlockingHydration: (fiber: Object) => void; -export function setAttemptHydration(fn: (fiber: Object) => void) { - attemptHydration = fn; +export function setAttemptUserBlockingHydration(fn: (fiber: Object) => void) { + attemptUserBlockingHydration = fn; } // TODO: Upgrade this definition once we're on a newer version of Flow that @@ -442,6 +442,12 @@ function replayUnblockedEvents() { let nextDiscreteEvent = queuedDiscreteEvents[0]; if (nextDiscreteEvent.blockedOn !== null) { // We're still blocked. + // Increase the priority of this boundary to unblock + // the next discrete event. + let fiber = getInstanceFromNode(nextDiscreteEvent.blockedOn); + if (fiber !== null) { + attemptUserBlockingHydration(fiber); + } break; } let nextBlockedOn = attemptToDispatchEvent( diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 1a04a1ffe5a0..e008d202ab07 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -78,7 +78,7 @@ import { current as ReactCurrentFiberCurrent, } from './ReactCurrentFiber'; import {StrictMode} from './ReactTypeOfMode'; -import {Sync} from './ReactFiberExpirationTime'; +import {Sync, computeInteractiveExpiration} from './ReactFiberExpirationTime'; import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig'; import { scheduleRefresh, @@ -384,7 +384,8 @@ export function attemptSynchronousHydration(fiber: Fiber): void { // If we're still blocked after this, we need to increase // the priority of any promises resolving within this // boundary so that they next attempt also has higher pri. - markRetryTimeIfNotHydrated(fiber, Sync); + let retryExpTime = computeInteractiveExpiration(requestCurrentTime()); + markRetryTimeIfNotHydrated(fiber, retryExpTime); break; } } @@ -407,7 +408,7 @@ function markRetryTimeIfNotHydrated(fiber: Fiber, retryTime: ExpirationTime) { } } -export function attemptHydration(fiber: Fiber): void { +export function attemptUserBlockingHydration(fiber: Fiber): void { if (fiber.tag !== SuspenseComponent) { // We ignore HostRoots here because we can't increase // their priority and they should not suspend on I/O, @@ -415,7 +416,9 @@ export function attemptHydration(fiber: Fiber): void { // Suspense. return; } - // TODO + let expTime = computeInteractiveExpiration(requestCurrentTime()); + scheduleWork(fiber, expTime); + markRetryTimeIfNotHydrated(fiber, expTime); } export {findHostInstance};