diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 33fadfd5396..25918e8b387 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -3397,6 +3397,13 @@ function updateSuspenseListComponent( let suspenseContext: SuspenseContext = suspenseStackCursor.current; + if (workInProgress.flags & DidCapture) { + // This is the second pass after having suspended in a row. Proceed directly + // to the complete phase. + pushSuspenseListContext(workInProgress, suspenseContext); + return null; + } + const shouldForceFallback = hasSuspenseListContext( suspenseContext, (ForceSuspenseFallback: SuspenseContext), @@ -4011,6 +4018,14 @@ function attemptEarlyBailoutIfNoScheduledUpdate( break; } case SuspenseListComponent: { + if (workInProgress.flags & DidCapture) { + // Second pass caught. + return updateSuspenseListComponent( + current, + workInProgress, + renderLanes, + ); + } const didSuspendBefore = (current.flags & DidCapture) !== NoFlags; let hasChildWork = includesSomeLane( diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 0b1f1e4366e..dab1b2272bd 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -138,6 +138,7 @@ import { popSuspenseListContext, popSuspenseHandler, pushSuspenseListContext, + pushSuspenseListCatch, setShallowSuspenseListContext, ForceSuspenseFallback, setDefaultShallowSuspenseListContext, @@ -765,6 +766,17 @@ function cutOffTailIfNeeded( } } +function isOnlyNewMounts(tail: Fiber): boolean { + let fiber: null | Fiber = tail; + while (fiber !== null) { + if (fiber.alternate !== null) { + return false; + } + fiber = fiber.sibling; + } + return true; +} + function bubbleProperties(completedWork: Fiber) { const didBailout = completedWork.alternate !== null && @@ -1855,7 +1867,10 @@ function completeWork( if (renderState.tail !== null) { // We still have tail rows to render. // Pop a row. + // TODO: Consider storing the first of the new mount tail in the state so + // that we don't have to recompute this for every row in the list. const next = renderState.tail; + const onlyNewMounts = isOnlyNewMounts(next); renderState.rendering = next; renderState.tail = next.sibling; renderState.renderingStartTime = now(); @@ -1874,7 +1889,26 @@ function completeWork( suspenseContext = setDefaultShallowSuspenseListContext(suspenseContext); } - pushSuspenseListContext(workInProgress, suspenseContext); + if ( + renderState.tailMode === 'visible' || + renderState.tailMode === 'collapsed' || + !onlyNewMounts || + // TODO: While hydrating, we still let it suspend the parent. Tail mode hidden has broken + // hydration anyway right now but this preserves the previous semantics out of caution. + // Once proper hydration is implemented, this special case should be removed as it should + // never be needed. + getIsHydrating() + ) { + pushSuspenseListContext(workInProgress, suspenseContext); + } else { + // If we are rendering in 'hidden' (default) tail mode, then we if we suspend in the + // tail itself, we can delete it rather than suspend the parent. So we act as a catch in that + // case. For 'collapsed' we need to render at least one in suspended state, after which we'll + // have cut off the rest to never attempt it so it never hits this case. + // If this is an updated node, we cannot delete it from the tail so it's effectively visible. + // As a consequence, if it resuspends it actually suspends the parent by taking the other path. + pushSuspenseListCatch(workInProgress, suspenseContext); + } // Do a pass over the next row. if (getIsHydrating()) { // Re-apply tree fork since we popped the tree fork context in the beginning of this function. diff --git a/packages/react-reconciler/src/ReactFiberSuspenseContext.js b/packages/react-reconciler/src/ReactFiberSuspenseContext.js index 3177b9d1b35..aec47af4656 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseContext.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseContext.js @@ -48,9 +48,10 @@ export function pushPrimaryTreeSuspenseHandler(handler: Fiber): void { // Shallow Suspense context fields, like ForceSuspenseFallback, should only be // propagated a single level. For example, when ForceSuspenseFallback is set, // it should only force the nearest Suspense boundary into fallback mode. - pushSuspenseListContext( - handler, + push( + suspenseStackCursor, setDefaultShallowSuspenseListContext(suspenseStackCursor.current), + handler, ); // Experimental feature: Some Suspense boundaries are marked as having an @@ -113,7 +114,7 @@ export function pushDehydratedActivitySuspenseHandler(fiber: Fiber): void { // Reuse the current value on the stack. // TODO: We can avoid needing to push here by by forking popSuspenseHandler // into separate functions for Activity, Suspense and Offscreen. - pushSuspenseListContext(fiber, suspenseStackCursor.current); + push(suspenseStackCursor, suspenseStackCursor.current, fiber); push(suspenseHandlerStackCursor, fiber, fiber); if (shellBoundary === null) { // We can contain any suspense inside the Activity boundary. @@ -127,7 +128,7 @@ export function pushOffscreenSuspenseHandler(fiber: Fiber): void { // Reuse the current value on the stack. // TODO: We can avoid needing to push here by by forking popSuspenseHandler // into separate functions for Activity, Suspense and Offscreen. - pushSuspenseListContext(fiber, suspenseStackCursor.current); + push(suspenseStackCursor, suspenseStackCursor.current, fiber); push(suspenseHandlerStackCursor, fiber, fiber); if (shellBoundary === null) { // We're rendering hidden content. If it suspends, we can handle it by @@ -141,7 +142,7 @@ export function pushOffscreenSuspenseHandler(fiber: Fiber): void { } export function reuseSuspenseHandlerOnStack(fiber: Fiber) { - pushSuspenseListContext(fiber, suspenseStackCursor.current); + push(suspenseStackCursor, suspenseStackCursor.current, fiber); push(suspenseHandlerStackCursor, getSuspenseHandler(), fiber); } @@ -155,7 +156,7 @@ export function popSuspenseHandler(fiber: Fiber): void { // Popping back into the shell. shellBoundary = null; } - popSuspenseListContext(fiber); + pop(suspenseStackCursor, fiber); } // SuspenseList context @@ -201,9 +202,32 @@ export function pushSuspenseListContext( fiber: Fiber, newContext: SuspenseContext, ): void { + // Push the current handler in this case since we're not catching at the SuspenseList + // for typical rows. + const handlerOnStack = suspenseHandlerStackCursor.current; + push(suspenseHandlerStackCursor, handlerOnStack, fiber); + push(suspenseStackCursor, newContext, fiber); +} + +export function pushSuspenseListCatch( + fiber: Fiber, + newContext: SuspenseContext, +): void { + // In this case we do want to handle catching suspending on the actual boundary itself. + // This is used for rows that are allowed to be hidden anyway. + push(suspenseHandlerStackCursor, fiber, fiber); push(suspenseStackCursor, newContext, fiber); + if (shellBoundary === null) { + // We can contain the effects to hiding the current row. + shellBoundary = fiber; + } } export function popSuspenseListContext(fiber: Fiber): void { pop(suspenseStackCursor, fiber); + pop(suspenseHandlerStackCursor, fiber); + if (shellBoundary === fiber) { + // Popping back into the shell. + shellBoundary = null; + } } diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 92540176fe8..8ff61b02697 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -27,6 +27,7 @@ import { ActivityComponent, SuspenseComponent, OffscreenComponent, + SuspenseListComponent, } from './ReactWorkTags'; import { DidCapture, @@ -400,7 +401,8 @@ function throwException( if (suspenseBoundary !== null) { switch (suspenseBoundary.tag) { case ActivityComponent: - case SuspenseComponent: { + case SuspenseComponent: + case SuspenseListComponent: { // If this suspense/activity boundary is not already showing a fallback, mark // the in-progress render as suspended. We try to perform this logic // as soon as soon as possible during the render phase, so the work @@ -561,6 +563,13 @@ function throwException( // Instead of surfacing the error, find the nearest Suspense boundary // and render it again without hydration. if (hydrationBoundary !== null) { + if (__DEV__) { + if (hydrationBoundary.tag === SuspenseListComponent) { + console.error( + 'SuspenseList should never catch while hydrating. This is a bug in React.', + ); + } + } if ((hydrationBoundary.flags & ShouldCapture) === NoFlags) { // Set a flag to indicate that we should try rendering the normal // children again, not the fallback. diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index efe08a43ac7..b2954c41f5a 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -11,7 +11,10 @@ import type {ReactContext} from 'shared/ReactTypes'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane'; import type {ActivityState} from './ReactFiberActivityComponent'; -import type {SuspenseState} from './ReactFiberSuspenseComponent'; +import type { + SuspenseState, + SuspenseListRenderState, +} from './ReactFiberSuspenseComponent'; import type {Cache} from './ReactFiberCacheComponent'; import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent'; @@ -31,7 +34,7 @@ import { CacheComponent, TracingMarkerComponent, } from './ReactWorkTags'; -import {DidCapture, NoFlags, ShouldCapture} from './ReactFiberFlags'; +import {DidCapture, NoFlags, ShouldCapture, Update} from './ReactFiberFlags'; import {NoMode, ProfileMode} from './ReactTypeOfMode'; import { enableProfilerTimer, @@ -180,8 +183,27 @@ function unwindWork( } case SuspenseListComponent: { popSuspenseListContext(workInProgress); - // SuspenseList doesn't actually catch anything. It should've been + // SuspenseList doesn't normally catch anything. It should've been // caught by a nested boundary. If not, it should bubble through. + const flags = workInProgress.flags; + if (flags & ShouldCapture) { + workInProgress.flags = (flags & ~ShouldCapture) | DidCapture; + // If we caught something on the SuspenseList itself it's because + // we want to ignore something. Re-enter the cycle and handle it + // in the complete phase. + const renderState: null | SuspenseListRenderState = + workInProgress.memoizedState; + if (renderState !== null) { + // Cut off any remaining tail work and don't commit the rendering one. + // This assumes that we have already confirmed that none of these are + // already mounted. + renderState.rendering = null; + renderState.tail = null; + } + // Schedule the commit phase to attach retry listeners. + workInProgress.flags |= Update; + return workInProgress; + } return null; } case HostPortal: diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 3de2ab6b16f..f7200458be1 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1355,7 +1355,7 @@ function finishConcurrentRender( throw new Error('Root did not complete. This is a bug in React.'); } case RootSuspendedWithDelay: { - if (!includesOnlyTransitions(lanes)) { + if (!includesOnlyTransitions(lanes) && !includesOnlyRetries(lanes)) { // Commit the placeholder. break; } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index ba5b668652f..c7d4de1421e 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -2346,6 +2346,227 @@ describe('ReactSuspenseList', () => { ); }); + // @gate enableSuspenseList + it('reveals "hidden" rows one by one without suspense boundaries', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + +
+ +
+ + +
+ ); + } + + ReactNoop.render( + + + , + ); + + await waitForAll(['Suspend! [A]']); + + // We can commit without any rows at all leaving empty. + expect(ReactNoop).toMatchRenderedOutput(null); + + await act(() => A.resolve()); + assertLog(['A', 'Suspend! [B]']); + + expect(ReactNoop).toMatchRenderedOutput( +
+ A +
, + ); + + await act(() => B.resolve()); + assertLog(['B', 'Suspend! [C]']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); + + expect(ReactNoop).toMatchRenderedOutput( + <> +
+ A +
+ B + , + ); + + await act(() => C.resolve()); + assertLog(['C']); + + expect(ReactNoop).toMatchRenderedOutput( + <> +
+ A +
+ B + C + , + ); + }); + + // @gate enableSuspenseList + it('preserves already mounted rows when a new hidden on is inserted in the tail', async () => { + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + let count = 0; + function MountCount({children}) { + // This component should only mount once. + React.useLayoutEffect(() => { + count++; + }, []); + return children; + } + + function Foo({insert}) { + return ( + + + {insert ? : null} + + }> + + + + + ); + } + + await act(() => { + ReactNoop.render(); + }); + assertLog(['A', 'Suspend! [C]', 'Loading C', 'Suspend! [C]']); + + expect(count).toBe(1); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading C + , + ); + + await act(() => { + ReactNoop.render(); + }); + + assertLog(['A', 'Suspend! [B]', 'A', 'Suspend! [B]']); + + expect(count).toBe(1); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading C + , + ); + + await act(async () => { + await B.resolve(); + await C.resolve(); + }); + + assertLog(['A', 'B', 'C']); + + expect(count).toBe(1); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); + }); + + // @gate enableSuspenseList + it('reveals "collapsed" rows one by one after the first without boundaries', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + +
+ }> + + + + + ); + } + + await act(async () => { + ReactNoop.render( + + + , + ); + await waitForAll(['Suspend! [A]', 'Suspend! [A]']); + }); + + // The root is still blocked on the first row. + expect(ReactNoop).toMatchRenderedOutput('Loading root'); + + await A.resolve(); + + await waitForAll(['A', 'Suspend! [B]', 'Loading B']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); + + // Because we have a Suspense boundary that can commit we can now unblock the rest. + // If it wasn't a boundary then we couldn't make progress because it would commit + // without any loading state. + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading B + , + ); + + await act(() => B.resolve()); + assertLog(['B', 'Suspend! [C]', 'B', 'Suspend! [C]']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); + + // Surprisingly unsuspending B actually causes the parent to resuspend + // because C is now unblocked which resuspends the parent. Preventing the + // Retry from committing. That's because we don't want to commit into a + // state that doesn't have any loading indicators at all. That's what + // "collapsed" is for. To ensure there's always a loading indicator. + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading B + , + ); + + await act(() => C.resolve()); + assertLog(['B', 'C']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); + }); + // @gate enableSuspenseList it('eventually resolves a nested forwards suspense list', async () => { const B = createAsyncText('B');