diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index b5271151a5414..c7326aaafa28c 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -9,10 +9,13 @@ import {REACT_STRICT_MODE_TYPE} from 'shared/ReactSymbols'; -import type {Wakeable} from 'shared/ReactTypes'; +import type {Wakeable, Thenable} from 'shared/ReactTypes'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.new'; -import type {SuspenseState} from './ReactFiberSuspenseComponent.new'; +import type { + SuspenseProps, + SuspenseState, +} from './ReactFiberSuspenseComponent.new'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new'; import type {EventPriority} from './ReactEventPriorities.new'; import type { @@ -271,6 +274,10 @@ import { isThenableStateResolved, } from './ReactFiberThenable.new'; import {schedulePostPaintCallback} from './ReactPostPaintCallback'; +import { + getSuspenseHandler, + isBadSuspenseFallback, +} from './ReactFiberSuspenseContext.new'; const ceil = Math.ceil; @@ -312,7 +319,7 @@ let workInProgressRootRenderLanes: Lanes = NoLanes; opaque type SuspendedReason = 0 | 1 | 2 | 3 | 4; const NotSuspended: SuspendedReason = 0; const SuspendedOnError: SuspendedReason = 1; -// const SuspendedOnData: SuspendedReason = 2; +const SuspendedOnData: SuspendedReason = 2; const SuspendedOnImmediate: SuspendedReason = 3; const SuspendedAndReadyToUnwind: SuspendedReason = 4; @@ -706,6 +713,18 @@ export function scheduleUpdateOnFiber( } } + // Check if the work loop is currently suspended and waiting for data to + // finish loading. + if ( + workInProgressSuspendedReason === SuspendedOnData && + root === workInProgressRoot + ) { + // The incoming update might unblock the current render. Interrupt the + // current attempt and restart from the top. + prepareFreshStack(root, NoLanes); + markRootSuspended(root, workInProgressRootRenderLanes); + } + // Mark that the root has a pending update. markRootUpdated(root, lane, eventTime); @@ -1130,6 +1149,20 @@ function performConcurrentWorkOnRoot(root, didTimeout) { if (root.callbackNode === originalCallbackNode) { // The task node scheduled for this root is the same one that's // currently executed. Need to return a continuation. + if ( + workInProgressSuspendedReason === SuspendedOnData && + workInProgressRoot === root + ) { + // Special case: The work loop is currently suspended and waiting for + // data to resolve. Unschedule the current task. + // + // TODO: The factoring is a little weird. Arguably this should be checked + // in ensureRootIsScheduled instead. I went back and forth, not totally + // sure yet. + root.callbackPriority = NoLane; + root.callbackNode = null; + return null; + } return performConcurrentWorkOnRoot.bind(null, root); } return null; @@ -1739,7 +1772,9 @@ function handleThrow(root, thrownValue): void { // deprecate the old API in favor of `use`. thrownValue = getSuspendedThenable(); workInProgressSuspendedThenableState = getThenableStateAfterSuspending(); - workInProgressSuspendedReason = SuspendedOnImmediate; + workInProgressSuspendedReason = shouldAttemptToSuspendUntilDataResolves() + ? SuspendedOnData + : SuspendedOnImmediate; } else { // This is a regular error. If something earlier in the component already // suspended, we must clear the thenable state to unblock the work loop. @@ -1796,6 +1831,48 @@ function handleThrow(root, thrownValue): void { } } +function shouldAttemptToSuspendUntilDataResolves() { + // TODO: We should be able to move the + // renderDidSuspend/renderDidSuspendWithDelay logic into this function, + // instead of repeating it in the complete phase. Or something to that effect. + + if (includesOnlyRetries(workInProgressRootRenderLanes)) { + // We can always wait during a retry. + return true; + } + + // TODO: We should be able to remove the equivalent check in + // finishConcurrentRender, and rely just on this one. + if (includesOnlyTransitions(workInProgressRootRenderLanes)) { + const suspenseHandler = getSuspenseHandler(); + if (suspenseHandler !== null && suspenseHandler.tag === SuspenseComponent) { + const currentSuspenseHandler = suspenseHandler.alternate; + const nextProps: SuspenseProps = suspenseHandler.memoizedProps; + if (isBadSuspenseFallback(currentSuspenseHandler, nextProps)) { + // The nearest Suspense boundary is already showing content. We should + // avoid replacing it with a fallback, and instead wait until the + // data finishes loading. + return true; + } else { + // This is not a bad fallback condition. We should show a fallback + // immediately instead of waiting for the data to resolve. This includes + // when suspending inside new trees. + return false; + } + } + + // During a transition, if there is no Suspense boundary (i.e. suspending in + // the "shell" of an application), or if we're inside a hidden tree, then + // we should wait until the data finishes loading. + return true; + } + + // For all other Lanes besides Transitions and Retries, we should not wait + // for the data to load. + // TODO: We should wait during Offscreen prerendering, too. + return false; +} + function pushDispatcher(container) { prepareRendererToRender(container); const prevDispatcher = ReactCurrentDispatcher.current; @@ -2060,7 +2137,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { markRenderStarted(lanes); } - do { + outer: do { try { if ( workInProgressSuspendedReason !== NotSuspended && @@ -2070,19 +2147,48 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // replay the suspended component. const unitOfWork = workInProgress; const thrownValue = workInProgressThrownValue; - workInProgressSuspendedReason = NotSuspended; - workInProgressThrownValue = null; switch (workInProgressSuspendedReason) { case SuspendedOnError: { // Unwind then continue with the normal work loop. + workInProgressSuspendedReason = NotSuspended; + workInProgressThrownValue = null; unwindSuspendedUnitOfWork(unitOfWork, thrownValue); break; } + case SuspendedOnData: { + const didResolve = + workInProgressSuspendedThenableState !== null && + isThenableStateResolved(workInProgressSuspendedThenableState); + if (didResolve) { + workInProgressSuspendedReason = NotSuspended; + workInProgressThrownValue = null; + replaySuspendedUnitOfWork(unitOfWork, thrownValue); + } else { + // The work loop is suspended on data. We should wait for it to + // resolve before continuing to render. + const thenable: Thenable = (workInProgressThrownValue: any); + const onResolution = () => { + ensureRootIsScheduled(root, now()); + }; + thenable.then(onResolution, onResolution); + break outer; + } + break; + } + case SuspendedOnImmediate: { + // If this fiber just suspended, it's possible the data is already + // cached. Yield to the main thread to give it a chance to ping. If + // it does, we can retry immediately without unwinding the stack. + workInProgressSuspendedReason = SuspendedAndReadyToUnwind; + break outer; + } default: { - const wasPinged = + workInProgressSuspendedReason = NotSuspended; + workInProgressThrownValue = null; + const didResolve = workInProgressSuspendedThenableState !== null && isThenableStateResolved(workInProgressSuspendedThenableState); - if (wasPinged) { + if (didResolve) { replaySuspendedUnitOfWork(unitOfWork, thrownValue); } else { unwindSuspendedUnitOfWork(unitOfWork, thrownValue); @@ -2096,12 +2202,6 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { break; } catch (thrownValue) { handleThrow(root, thrownValue); - if (workInProgressSuspendedThenableState !== null) { - // If this fiber just suspended, it's possible the data is already - // cached. Yield to the main thread to give it a chance to ping. If - // it does, we can retry immediately without unwinding the stack. - break; - } } } while (true); resetContextDependencies(); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 4510f2a31f58f..04f835ee9bf56 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -9,10 +9,13 @@ import {REACT_STRICT_MODE_TYPE} from 'shared/ReactSymbols'; -import type {Wakeable} from 'shared/ReactTypes'; +import type {Wakeable, Thenable} from 'shared/ReactTypes'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.old'; -import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; +import type { + SuspenseProps, + SuspenseState, +} from './ReactFiberSuspenseComponent.old'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old'; import type {EventPriority} from './ReactEventPriorities.old'; import type { @@ -271,6 +274,10 @@ import { isThenableStateResolved, } from './ReactFiberThenable.old'; import {schedulePostPaintCallback} from './ReactPostPaintCallback'; +import { + getSuspenseHandler, + isBadSuspenseFallback, +} from './ReactFiberSuspenseContext.old'; const ceil = Math.ceil; @@ -312,7 +319,7 @@ let workInProgressRootRenderLanes: Lanes = NoLanes; opaque type SuspendedReason = 0 | 1 | 2 | 3 | 4; const NotSuspended: SuspendedReason = 0; const SuspendedOnError: SuspendedReason = 1; -// const SuspendedOnData: SuspendedReason = 2; +const SuspendedOnData: SuspendedReason = 2; const SuspendedOnImmediate: SuspendedReason = 3; const SuspendedAndReadyToUnwind: SuspendedReason = 4; @@ -706,6 +713,18 @@ export function scheduleUpdateOnFiber( } } + // Check if the work loop is currently suspended and waiting for data to + // finish loading. + if ( + workInProgressSuspendedReason === SuspendedOnData && + root === workInProgressRoot + ) { + // The incoming update might unblock the current render. Interrupt the + // current attempt and restart from the top. + prepareFreshStack(root, NoLanes); + markRootSuspended(root, workInProgressRootRenderLanes); + } + // Mark that the root has a pending update. markRootUpdated(root, lane, eventTime); @@ -1130,6 +1149,20 @@ function performConcurrentWorkOnRoot(root, didTimeout) { if (root.callbackNode === originalCallbackNode) { // The task node scheduled for this root is the same one that's // currently executed. Need to return a continuation. + if ( + workInProgressSuspendedReason === SuspendedOnData && + workInProgressRoot === root + ) { + // Special case: The work loop is currently suspended and waiting for + // data to resolve. Unschedule the current task. + // + // TODO: The factoring is a little weird. Arguably this should be checked + // in ensureRootIsScheduled instead. I went back and forth, not totally + // sure yet. + root.callbackPriority = NoLane; + root.callbackNode = null; + return null; + } return performConcurrentWorkOnRoot.bind(null, root); } return null; @@ -1739,7 +1772,9 @@ function handleThrow(root, thrownValue): void { // deprecate the old API in favor of `use`. thrownValue = getSuspendedThenable(); workInProgressSuspendedThenableState = getThenableStateAfterSuspending(); - workInProgressSuspendedReason = SuspendedOnImmediate; + workInProgressSuspendedReason = shouldAttemptToSuspendUntilDataResolves() + ? SuspendedOnData + : SuspendedOnImmediate; } else { // This is a regular error. If something earlier in the component already // suspended, we must clear the thenable state to unblock the work loop. @@ -1796,6 +1831,48 @@ function handleThrow(root, thrownValue): void { } } +function shouldAttemptToSuspendUntilDataResolves() { + // TODO: We should be able to move the + // renderDidSuspend/renderDidSuspendWithDelay logic into this function, + // instead of repeating it in the complete phase. Or something to that effect. + + if (includesOnlyRetries(workInProgressRootRenderLanes)) { + // We can always wait during a retry. + return true; + } + + // TODO: We should be able to remove the equivalent check in + // finishConcurrentRender, and rely just on this one. + if (includesOnlyTransitions(workInProgressRootRenderLanes)) { + const suspenseHandler = getSuspenseHandler(); + if (suspenseHandler !== null && suspenseHandler.tag === SuspenseComponent) { + const currentSuspenseHandler = suspenseHandler.alternate; + const nextProps: SuspenseProps = suspenseHandler.memoizedProps; + if (isBadSuspenseFallback(currentSuspenseHandler, nextProps)) { + // The nearest Suspense boundary is already showing content. We should + // avoid replacing it with a fallback, and instead wait until the + // data finishes loading. + return true; + } else { + // This is not a bad fallback condition. We should show a fallback + // immediately instead of waiting for the data to resolve. This includes + // when suspending inside new trees. + return false; + } + } + + // During a transition, if there is no Suspense boundary (i.e. suspending in + // the "shell" of an application), or if we're inside a hidden tree, then + // we should wait until the data finishes loading. + return true; + } + + // For all other Lanes besides Transitions and Retries, we should not wait + // for the data to load. + // TODO: We should wait during Offscreen prerendering, too. + return false; +} + function pushDispatcher(container) { prepareRendererToRender(container); const prevDispatcher = ReactCurrentDispatcher.current; @@ -2060,7 +2137,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { markRenderStarted(lanes); } - do { + outer: do { try { if ( workInProgressSuspendedReason !== NotSuspended && @@ -2070,19 +2147,48 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { // replay the suspended component. const unitOfWork = workInProgress; const thrownValue = workInProgressThrownValue; - workInProgressSuspendedReason = NotSuspended; - workInProgressThrownValue = null; switch (workInProgressSuspendedReason) { case SuspendedOnError: { // Unwind then continue with the normal work loop. + workInProgressSuspendedReason = NotSuspended; + workInProgressThrownValue = null; unwindSuspendedUnitOfWork(unitOfWork, thrownValue); break; } + case SuspendedOnData: { + const didResolve = + workInProgressSuspendedThenableState !== null && + isThenableStateResolved(workInProgressSuspendedThenableState); + if (didResolve) { + workInProgressSuspendedReason = NotSuspended; + workInProgressThrownValue = null; + replaySuspendedUnitOfWork(unitOfWork, thrownValue); + } else { + // The work loop is suspended on data. We should wait for it to + // resolve before continuing to render. + const thenable: Thenable = (workInProgressThrownValue: any); + const onResolution = () => { + ensureRootIsScheduled(root, now()); + }; + thenable.then(onResolution, onResolution); + break outer; + } + break; + } + case SuspendedOnImmediate: { + // If this fiber just suspended, it's possible the data is already + // cached. Yield to the main thread to give it a chance to ping. If + // it does, we can retry immediately without unwinding the stack. + workInProgressSuspendedReason = SuspendedAndReadyToUnwind; + break outer; + } default: { - const wasPinged = + workInProgressSuspendedReason = NotSuspended; + workInProgressThrownValue = null; + const didResolve = workInProgressSuspendedThenableState !== null && isThenableStateResolved(workInProgressSuspendedThenableState); - if (wasPinged) { + if (didResolve) { replaySuspendedUnitOfWork(unitOfWork, thrownValue); } else { unwindSuspendedUnitOfWork(unitOfWork, thrownValue); @@ -2096,12 +2202,6 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { break; } catch (thrownValue) { handleThrow(root, thrownValue); - if (workInProgressSuspendedThenableState !== null) { - // If this fiber just suspended, it's possible the data is already - // cached. Yield to the main thread to give it a chance to ping. If - // it does, we can retry immediately without unwinding the stack. - break; - } } } while (true); resetContextDependencies(); diff --git a/packages/react-reconciler/src/__tests__/ReactThenable-test.js b/packages/react-reconciler/src/__tests__/ReactThenable-test.js index fd95a43bfab8f..887cd26080d8f 100644 --- a/packages/react-reconciler/src/__tests__/ReactThenable-test.js +++ b/packages/react-reconciler/src/__tests__/ReactThenable-test.js @@ -7,6 +7,7 @@ let act; let use; let Suspense; let startTransition; +let pendingTextRequests; describe('ReactThenable', () => { beforeEach(() => { @@ -19,11 +20,37 @@ describe('ReactThenable', () => { use = React.use; Suspense = React.Suspense; startTransition = React.startTransition; + + pendingTextRequests = new Map(); }); - function Text(props) { - Scheduler.unstable_yieldValue(props.text); - return props.text; + function resolveTextRequests(text) { + const requests = pendingTextRequests.get(text); + if (requests !== undefined) { + pendingTextRequests.delete(text); + requests.forEach(resolve => resolve(text)); + } + } + + function getAsyncText(text) { + // getAsyncText is completely uncached — it performs a new async operation + // every time it's called. During a transition, React should be able to + // unwrap it anyway. + Scheduler.unstable_yieldValue(`Async text requested [${text}]`); + return new Promise(resolve => { + const requests = pendingTextRequests.get(text); + if (requests !== undefined) { + requests.push(resolve); + pendingTextRequests.set(text, requests); + } else { + pendingTextRequests.set(text, [resolve]); + } + }); + } + + function Text({text}) { + Scheduler.unstable_yieldValue(text); + return text; } // This behavior was intentionally disabled to derisk the rollout of `use`. @@ -32,15 +59,15 @@ describe('ReactThenable', () => { // to `use`, so the extra code probably isn't worth it. // @gate TODO test('if suspended fiber is pinged in a microtask, retry immediately without unwinding the stack', async () => { - let resolved = false; + let fulfilled = false; function Async() { - if (resolved) { + if (fulfilled) { return ; } Scheduler.unstable_yieldValue('Suspend!'); throw Promise.resolve().then(() => { Scheduler.unstable_yieldValue('Resolve in microtask'); - resolved = true; + fulfilled = true; }); } @@ -71,15 +98,15 @@ describe('ReactThenable', () => { }); test('if suspended fiber is pinged in a microtask, it does not block a transition from completing', async () => { - let resolved = false; + let fulfilled = false; function Async() { - if (resolved) { + if (fulfilled) { return ; } Scheduler.unstable_yieldValue('Suspend!'); throw Promise.resolve().then(() => { Scheduler.unstable_yieldValue('Resolve in microtask'); - resolved = true; + fulfilled = true; }); } @@ -101,13 +128,13 @@ describe('ReactThenable', () => { expect(root).toMatchRenderedOutput('Async'); }); - test('does not infinite loop if already resolved thenable is thrown', async () => { - // An already resolved promise should never be thrown. Since it already - // resolved, we shouldn't bother trying to render again — doing so would + test('does not infinite loop if already fulfilled thenable is thrown', async () => { + // An already fulfilled promise should never be thrown. Since it already + // fulfilled, we shouldn't bother trying to render again — doing so would // likely lead to an infinite loop. This scenario should only happen if a // userspace Suspense library makes an implementation mistake. - // Create an already resolved thenable + // Create an already fulfilled thenable const thenable = { then(ping) {}, status: 'fulfilled', @@ -120,7 +147,7 @@ describe('ReactThenable', () => { throw new Error('Infinite loop detected'); } Scheduler.unstable_yieldValue('Suspend!'); - // This thenable should never be thrown because it already resolved. + // This thenable should never be thrown because it already fulfilled. // But if it is thrown, React should handle it gracefully. throw thenable; } @@ -365,7 +392,7 @@ describe('ReactThenable', () => { expect(Scheduler).toHaveYielded([ // First attempt. The uncached promise suspends. 'Suspend! [Async]', - // Because the promise already resolved, we're able to unwrap the value + // Because the promise already fulfilled, we're able to unwrap the value // immediately in a microtask. // // Then we proceed to the rest of the component, which throws an error. @@ -497,4 +524,120 @@ describe('ReactThenable', () => { ); } }); + + // @gate enableUseHook + test('during a transition, can unwrap async operations even if nothing is cached', async () => { + function App() { + return ; + } + + const root = ReactNoop.createRoot(); + await act(async () => { + root.render( + }> + + , + ); + }); + expect(Scheduler).toHaveYielded(['(empty)']); + expect(root).toMatchRenderedOutput('(empty)'); + + await act(async () => { + startTransition(() => { + root.render( + }> + + , + ); + }); + }); + expect(Scheduler).toHaveYielded(['Async text requested [Async]']); + expect(root).toMatchRenderedOutput('(empty)'); + + await act(async () => { + resolveTextRequests('Async'); + }); + expect(Scheduler).toHaveYielded(['Async text requested [Async]', 'Async']); + expect(root).toMatchRenderedOutput('Async'); + }); + + // @gate enableUseHook + test("does not prevent a Suspense fallback from showing if it's a new boundary, even during a transition", async () => { + function App() { + return ; + } + + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => { + root.render( + }> + + , + ); + }); + }); + // Even though the initial render was a transition, it shows a fallback. + expect(Scheduler).toHaveYielded([ + 'Async text requested [Async]', + 'Loading...', + ]); + expect(root).toMatchRenderedOutput('Loading...'); + + // Resolve the original data + await act(async () => { + resolveTextRequests('Async'); + }); + // During the retry, a fresh request is initiated. Now we must wait for this + // one to finish. + // TODO: This is awkward. Intuitively, you might expect for `act` to wait + // until the new request has finished loading. But if it's mock IO, as in + // this test, how would the developer be able to imperatively flush it if it + // wasn't initiated until the current `act` call? Can't think of a better + // strategy at the moment. + expect(Scheduler).toHaveYielded(['Async text requested [Async]']); + expect(root).toMatchRenderedOutput('Loading...'); + + // Flush the second request. + await act(async () => { + resolveTextRequests('Async'); + }); + // This time it finishes because it was during a retry. + expect(Scheduler).toHaveYielded(['Async text requested [Async]', 'Async']); + expect(root).toMatchRenderedOutput('Async'); + }); + + // @gate enableUseHook + test('when waiting for data to resolve, a fresh update will trigger a restart', async () => { + function App() { + return ; + } + + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(} />); + }); + + await act(async () => { + startTransition(() => { + root.render( + }> + + , + ); + }); + }); + expect(Scheduler).toHaveYielded([ + 'Async text requested [Will never resolve]', + ]); + + await act(async () => { + root.render( + }> + + , + ); + }); + expect(Scheduler).toHaveYielded(['Something different']); + }); });