From ec94d67be8bb1d0f3c85a770a3b8dfffab8eeb94 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Fri, 9 Dec 2022 19:44:54 -0800 Subject: [PATCH 1/7] Add a regression test for Force unwind work loop during selective hydration #25695 --- ...MServerSelectiveHydration-test.internal.js | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 3469c4faf0683..c1adcb01000a3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1772,4 +1772,71 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.removeChild(container); }); + + it('regression test: can unwind context on selective hydration interruption', async () => { + const Context = React.createContext('DefaultContext'); + + function ContextReader(props) { + const value = React.useContext(Context); + Scheduler.unstable_yieldValue(value); + return null; + } + + function Child({text}) { + Scheduler.unstable_yieldValue(text); + return {text}; + } + const ChildWithBoundary = React.memo(function({text}) { + return ( + + + + ); + }); + + function App({a}) { + Scheduler.unstable_yieldValue('App'); + React.useEffect(() => { + Scheduler.unstable_yieldValue('Commit'); + }); + return ( + <> + + + + + + ); + } + const finalHTML = ReactDOMServer.renderToString(); + expect(Scheduler).toHaveYielded(['App', 'A', 'DefaultContext']); + const container = document.createElement('div'); + container.innerHTML = finalHTML; + document.body.appendChild(container); + + const spanA = container.getElementsByTagName('span')[0]; + + await act(async () => { + const root = ReactDOMClient.hydrateRoot(container, ); + expect(Scheduler).toFlushAndYieldThrough([ + 'App', + 'DefaultContext', + 'Commit', + ]); + + TODO_scheduleIdleDOMSchedulerTask(() => { + root.render(); + }); + expect(Scheduler).toFlushAndYieldThrough(['App', 'AA', 'DefaultContext']); + + dispatchClickEvent(spanA); + expect(Scheduler).toHaveYielded(['A']); + expect(Scheduler).toFlushAndYield([ + 'App', + 'AA', + 'DefaultContext', + 'Commit', + ]); + }); + }); }); From 12b14c48673e5a7337ce5cda00a556691d6a24b6 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Fri, 9 Dec 2022 19:55:30 -0800 Subject: [PATCH 2/7] add gate --- .../__tests__/ReactDOMServerSelectiveHydration-test.internal.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index c1adcb01000a3..2a09adec2f0cb 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1773,6 +1773,7 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.removeChild(container); }); + // @gate experimental || www it('regression test: can unwind context on selective hydration interruption', async () => { const Context = React.createContext('DefaultContext'); From e2f4ff3a8c855a452153e6f44ddf566fdf7a2f07 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 17 Nov 2022 13:51:33 -0500 Subject: [PATCH 3/7] Force unwind work loop during selective hydration (#25695) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an update flows into a dehydrated boundary, React cannot apply the update until the boundary has finished hydrating. The way this currently works is by scheduling a slightly higher priority task on the boundary, using a special lane that's reserved only for this purpose. Because the task is slightly higher priority, on the next turn of the work loop, the Scheduler will force the work loop to yield (i.e. shouldYield starts returning `true` because there's a higher priority task). The downside of this approach is that it only works when time slicing is enabled. It doesn't work for synchronous updates, because the synchronous work loop does not consult the Scheduler on each iteration. We plan to add support for selective hydration during synchronous updates, too, so we need to model this some other way. I've added a special internal exception that can be thrown to force the work loop to interrupt the work-in-progress tree. Because it's thrown from a React-only execution stack, throwing isn't strictly necessary — we could instead modify some internal work loop state. But using an exception means we don't need to check for this case on every iteration of the work loop. So doing it this way moves the check out of the fast path. The ideal implementation wouldn't need to unwind the stack at all — we should be able to hydrate the subtree and then apply the update all within a single render phase. This is how we intend to implement it in the future, but this requires a refactor to how we handle "stack" variables, which are currently pushed to a per-render array. We need to make this stack resumable, like how context works in Flight and Fizz. --- ...MServerSelectiveHydration-test.internal.js | 8 ++-- .../src/ReactFiberBeginWork.js | 32 ++++++++++--- .../src/ReactFiberWorkLoop.js | 47 ++++++++++++++++--- 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 2a09adec2f0cb..6b208f257b6f0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1496,12 +1496,10 @@ describe('ReactDOMServerSelectiveHydration', () => { // Start rendering. This will force the first boundary to hydrate // by scheduling it at one higher pri than Idle. expect(Scheduler).toFlushAndYieldThrough([ - // An update was scheduled to force hydrate the boundary, but React will - // continue rendering at Idle until the next time React yields. This is - // fine though because it will switch to the hydration level when it - // re-enters the work loop. 'App', - 'AA', + + // Start hydrating A + 'A', ]); // Hover over A which (could) schedule at one higher pri than Idle. diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 78372d1762916..bde65936f1c01 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -282,6 +282,14 @@ import { const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; +// A special exception that's used to unwind the stack when an update flows +// into a dehydrated boundary. +export const SelectiveHydrationException: mixed = new Error( + "This is not a real error. It's an implementation detail of React's " + + "selective hydration feature. If this leaks into userspace, it's a bug in " + + 'React. Please file an issue.', +); + let didReceiveUpdate: boolean = false; let didWarnAboutBadClass; @@ -2860,6 +2868,16 @@ function updateDehydratedSuspenseComponent( attemptHydrationAtLane, eventTime, ); + + // Throw a special object that signals to the work loop that it should + // interrupt the current render. + // + // Because we're inside a React-only execution stack, we don't + // strictly need to throw here — we could instead modify some internal + // work loop state. But using an exception means we don't need to + // check for this case on every iteration of the work loop. So doing + // it this way moves the check out of the fast path. + throw SelectiveHydrationException; } else { // We have already tried to ping at a higher priority than we're rendering with // so if we got here, we must have failed to hydrate at those levels. We must @@ -2870,15 +2888,17 @@ function updateDehydratedSuspenseComponent( } } - // If we have scheduled higher pri work above, this will just abort the render - // since we now have higher priority work. We'll try to infinitely suspend until - // we yield. TODO: We could probably just force yielding earlier instead. - renderDidSuspendDelayIfPossible(); - // If we rendered synchronously, we won't yield so have to render something. - // This will cause us to delete any existing content. + // If we did not selectively hydrate, we'll continue rendering without + // hydrating. Mark this tree as suspended to prevent it from committing + // outside a transition. + // + // This path should only happen if the hydration lane already suspended. + // Currently, it also happens during sync updates because there is no + // hydration lane for sync updates. // TODO: We should ideally have a sync hydration lane that we can apply to do // a pass where we hydrate this subtree in place using the previous Context and then // reapply the update afterwards. + renderDidSuspendDelayIfPossible(); return retrySuspenseComponentWithoutHydrating( current, workInProgress, diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 0c86cbb324020..a9e9a078e8aa9 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -175,6 +175,7 @@ import { } from './ReactEventPriorities'; import {requestCurrentTransition, NoTransition} from './ReactFiberTransition'; import { + SelectiveHydrationException, beginWork as originalBeginWork, replayFunctionComponent, } from './ReactFiberBeginWork'; @@ -316,13 +317,14 @@ let workInProgress: Fiber | null = null; // The lanes we're rendering let workInProgressRootRenderLanes: Lanes = NoLanes; -opaque type SuspendedReason = 0 | 1 | 2 | 3 | 4 | 5; +opaque type SuspendedReason = 0 | 1 | 2 | 3 | 4 | 5 | 6; const NotSuspended: SuspendedReason = 0; const SuspendedOnError: SuspendedReason = 1; const SuspendedOnData: SuspendedReason = 2; const SuspendedOnImmediate: SuspendedReason = 3; const SuspendedOnDeprecatedThrowPromise: SuspendedReason = 4; const SuspendedAndReadyToUnwind: SuspendedReason = 5; +const SuspendedOnHydration: SuspendedReason = 6; // When this is true, the work-in-progress fiber just suspended (or errored) and // we've yet to unwind the stack. In some cases, we may yield to the main thread @@ -1797,6 +1799,17 @@ function handleThrow(root, thrownValue): void { workInProgressSuspendedReason = shouldAttemptToSuspendUntilDataResolves() ? SuspendedOnData : SuspendedOnImmediate; + } else if (thrownValue === SelectiveHydrationException) { + // An update flowed into a dehydrated boundary. Before we can apply the + // update, we need to finish hydrating. Interrupt the work-in-progress + // render so we can restart at the hydration lane. + // + // The ideal implementation would be able to switch contexts without + // unwinding the current stack. + // + // We could name this something more general but as of now it's the only + // case where we think this should happen. + workInProgressSuspendedReason = SuspendedOnHydration; } else { // This is a regular error. const isWakeable = @@ -2038,7 +2051,7 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { markRenderStarted(lanes); } - do { + outer: do { try { if ( workInProgressSuspendedReason !== NotSuspended && @@ -2054,11 +2067,23 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { // function and fork the behavior some other way. const unitOfWork = workInProgress; const thrownValue = workInProgressThrownValue; - workInProgressSuspendedReason = NotSuspended; - workInProgressThrownValue = null; - unwindSuspendedUnitOfWork(unitOfWork, thrownValue); - - // Continue with the normal work loop. + switch (workInProgressSuspendedReason) { + case SuspendedOnHydration: { + // Selective hydration. An update flowed into a dehydrated tree. + // Interrupt the current render so the work loop can switch to the + // hydration lane. + workInProgress = null; + workInProgressRootExitStatus = RootDidNotComplete; + break outer; + } + default: { + // Continue with the normal work loop. + workInProgressSuspendedReason = NotSuspended; + workInProgressThrownValue = null; + unwindSuspendedUnitOfWork(unitOfWork, thrownValue); + break; + } + } } workLoopSync(); break; @@ -2216,6 +2241,14 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { unwindSuspendedUnitOfWork(unitOfWork, thrownValue); break; } + case SuspendedOnHydration: { + // Selective hydration. An update flowed into a dehydrated tree. + // Interrupt the current render so the work loop can switch to the + // hydration lane. + workInProgress = null; + workInProgressRootExitStatus = RootDidNotComplete; + break outer; + } default: { throw new Error( 'Unexpected SuspendedReason. This is a bug in React.', From b37a68a83247975d1e74bdba88768c0b80b8dcee Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Tue, 6 Dec 2022 18:48:07 -0800 Subject: [PATCH 4/7] Avoid replaying SelectiveHydrationException in dev --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index a9e9a078e8aa9..6f86c38a9bc20 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3774,6 +3774,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { if ( didSuspendOrErrorWhileHydratingDEV() || originalError === SuspenseException || + originalError === SelectiveHydrationException || (originalError !== null && typeof originalError === 'object' && typeof originalError.then === 'function') From 7e6e61acabafb109439744083536e52c379b0ab7 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Mon, 12 Dec 2022 17:59:42 -0800 Subject: [PATCH 5/7] Fix unwinding context during selective hydration --- .../ReactDOMServerSelectiveHydration-test.internal.js | 2 +- packages/react-reconciler/src/ReactFiberWorkLoop.js | 3 ++- 2 files changed, 3 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 6b208f257b6f0..f4483e9bca8f5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1826,7 +1826,7 @@ describe('ReactDOMServerSelectiveHydration', () => { TODO_scheduleIdleDOMSchedulerTask(() => { root.render(); }); - expect(Scheduler).toFlushAndYieldThrough(['App', 'AA', 'DefaultContext']); + expect(Scheduler).toFlushAndYieldThrough(['App', 'A']); dispatchClickEvent(spanA); expect(Scheduler).toHaveYielded(['A']); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 6f86c38a9bc20..8e78c7ccbc238 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2245,8 +2245,9 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Selective hydration. An update flowed into a dehydrated tree. // Interrupt the current render so the work loop can switch to the // hydration lane. - workInProgress = null; workInProgressRootExitStatus = RootDidNotComplete; + // `workInProgress` is not set to `null` to allow unwinding in + // `prepareFreshStack`. break outer; } default: { From b7c8958e9611656de0523f7b6c7853418f1b54f1 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Wed, 14 Dec 2022 12:25:17 -0800 Subject: [PATCH 6/7] Reset stack on interruption --- .../src/ReactFiberWorkLoop.js | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 8e78c7ccbc238..88e8577ad6696 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1703,6 +1703,30 @@ export function getRenderLanes(): Lanes { return renderLanes; } +function resetWorkInProgressStack() { + if (workInProgress == null) return; + let interruptedWork; + if (workInProgressSuspendedReason === NotSuspended) { + // Normal case. Work-in-progress hasn't started yet. Unwind all + // its parents. + interruptedWork = workInProgress.return; + } else { + // Work-in-progress is in suspended state. Reset the work loop and unwind + // both the suspended fiber and all its parents. + resetSuspendedWorkLoopOnUnwind(); + interruptedWork = workInProgress; + } + while (interruptedWork !== null) { + const current = interruptedWork.alternate; + unwindInterruptedWork( + current, + interruptedWork, + workInProgressRootRenderLanes, + ); + interruptedWork = interruptedWork.return; + } +} + function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { root.finishedWork = null; root.finishedLanes = NoLanes; @@ -1716,28 +1740,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { cancelTimeout(timeoutHandle); } - if (workInProgress !== null) { - let interruptedWork; - if (workInProgressSuspendedReason === NotSuspended) { - // Normal case. Work-in-progress hasn't started yet. Unwind all - // its parents. - interruptedWork = workInProgress.return; - } else { - // Work-in-progress is in suspended state. Reset the work loop and unwind - // both the suspended fiber and all its parents. - resetSuspendedWorkLoopOnUnwind(); - interruptedWork = workInProgress; - } - while (interruptedWork !== null) { - const current = interruptedWork.alternate; - unwindInterruptedWork( - current, - interruptedWork, - workInProgressRootRenderLanes, - ); - interruptedWork = interruptedWork.return; - } - } + resetWorkInProgressStack(); workInProgressRoot = root; const rootWorkInProgress = createWorkInProgress(root.current, null); workInProgress = rootWorkInProgress; @@ -2245,9 +2248,9 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // Selective hydration. An update flowed into a dehydrated tree. // Interrupt the current render so the work loop can switch to the // hydration lane. + resetWorkInProgressStack(); workInProgressRootExitStatus = RootDidNotComplete; - // `workInProgress` is not set to `null` to allow unwinding in - // `prepareFreshStack`. + workInProgress = null; break outer; } default: { From c27ac20931d6b304e168ed886a510f5a242fd01a Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Wed, 14 Dec 2022 15:36:53 -0800 Subject: [PATCH 7/7] null workInProgress in the reset function --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 88e8577ad6696..2a1166cc43c1a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1704,7 +1704,7 @@ export function getRenderLanes(): Lanes { } function resetWorkInProgressStack() { - if (workInProgress == null) return; + if (workInProgress === null) return; let interruptedWork; if (workInProgressSuspendedReason === NotSuspended) { // Normal case. Work-in-progress hasn't started yet. Unwind all @@ -1725,6 +1725,7 @@ function resetWorkInProgressStack() { ); interruptedWork = interruptedWork.return; } + workInProgress = null; } function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { @@ -2075,7 +2076,7 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { // Selective hydration. An update flowed into a dehydrated tree. // Interrupt the current render so the work loop can switch to the // hydration lane. - workInProgress = null; + resetWorkInProgressStack(); workInProgressRootExitStatus = RootDidNotComplete; break outer; } @@ -2250,7 +2251,6 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // hydration lane. resetWorkInProgressStack(); workInProgressRootExitStatus = RootDidNotComplete; - workInProgress = null; break outer; } default: {