diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 3be7abfa2a..e0fde1fcf9 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -127,6 +127,7 @@ import { addSubtreeSuspenseContext, setShallowSuspenseContext, } from './ReactFiberSuspenseContext'; +import {isShowingAnyFallbacks} from './ReactFiberSuspenseComponent'; import { pushProvider, propagateContextChange, @@ -1983,8 +1984,36 @@ function propagateSuspenseContextChange( } } -type SuspenseListRevealOrder = 'together' | void; +function findLastContentRow(firstChild: null | Fiber): null | Fiber { + // This is going to find the last row among these children that is already + // showing content on the screen, as opposed to being in fallback state or + // new. If a row has multiple Suspense boundaries, any of them being in the + // fallback state, counts as the whole row being in a fallback state. + // Note that the "rows" will be workInProgress, but any nested children + // will still be current since we haven't rendered them yet. The mounted + // order may not be the same as the new order. We use the new order. + let row = firstChild; + let lastContentRow: null | Fiber = null; + while (row !== null) { + let currentRow = row.alternate; + // New rows can't be content rows. + if (currentRow !== null && !isShowingAnyFallbacks(currentRow)) { + lastContentRow = row; + } + row = row.sibling; + } + return lastContentRow; +} +type SuspenseListRevealOrder = 'forwards' | 'backwards' | 'together' | void; + +// This can end up rendering this component multiple passes. +// The first pass splits the children fibers into two sets. A head and tail. +// We first render the head. If anything is in fallback state, we do another +// pass through beginWork to rerender all children (including the tail) with +// the force suspend context. If the first render didn't have anything in +// in fallback state. Then we render each row in the tail one-by-one. +// That happens in the completeWork phase without going back to beginWork. function updateSuspenseListComponent( current: Fiber | null, workInProgress: Fiber, @@ -2033,6 +2062,10 @@ function updateSuspenseListComponent( ); suspenseListState = { didSuspend: true, + isBackwards: false, + rendering: null, + last: null, + tail: null, }; } else { let didForceFallback = @@ -2063,7 +2096,79 @@ function updateSuspenseListComponent( } switch (revealOrder) { - // TODO: For other reveal orders we'll need to split the nextChildFibers set. + case 'forwards': { + // If need to force fallbacks in this pass we're just going to + // force the whole set to suspend so we don't have to do anything + // further here. + if (!shouldForceFallback) { + let lastContentRow = findLastContentRow(nextChildFibers); + let tail; + if (lastContentRow === null) { + // The whole list is part of the tail. + // TODO: We could fast path by just rendering the tail now. + tail = nextChildFibers; + nextChildFibers = null; + } else { + // Disconnect the tail rows after the content row. + // We're going to render them separately later. + tail = lastContentRow.sibling; + lastContentRow.sibling = null; + } + if (suspenseListState === null) { + suspenseListState = { + didSuspend: false, + isBackwards: false, + rendering: null, + last: lastContentRow, + tail: tail, + }; + } else { + suspenseListState.tail = tail; + } + } + break; + } + case 'backwards': { + // If need to force fallbacks in this pass we're just going to + // force the whole set to suspend so we don't have to do anything + // further here. + if (!shouldForceFallback) { + // We're going to find the first row that has existing content. + // At the same time we're going to reverse the list of everything + // we pass in the meantime. That's going to be our tail in reverse + // order. + let tail = null; + let row = nextChildFibers; + nextChildFibers = null; + while (row !== null) { + let currentRow = row.alternate; + // New rows can't be content rows. + if (currentRow !== null && !isShowingAnyFallbacks(currentRow)) { + // This is the beginning of the main content. + nextChildFibers = row; + break; + } + let nextRow = row.sibling; + row.sibling = tail; + tail = row; + row = nextRow; + } + // TODO: If nextChildFibers is null, we can continue on the tail immediately. + if (suspenseListState === null) { + suspenseListState = { + didSuspend: false, + isBackwards: true, + rendering: null, + last: null, + tail: tail, + }; + } else { + suspenseListState.isBackwards = true; + suspenseListState.tail = tail; + } + } + break; + } case 'together': { break; } @@ -2076,12 +2181,48 @@ function updateSuspenseListComponent( !didWarnAboutRevealOrder[revealOrder] ) { didWarnAboutRevealOrder[revealOrder] = true; - warning( - false, - '"%s" is not a supported revealOrder on . ' + - 'Did you mean "together"?', - revealOrder, - ); + if (typeof revealOrder === 'string') { + switch (revealOrder.toLowerCase()) { + case 'together': + case 'forwards': + case 'backwards': { + warning( + false, + '"%s" is not a valid value for revealOrder on . ' + + 'Use lowercase "%s" instead.', + revealOrder, + revealOrder.toLowerCase(), + ); + break; + } + case 'forward': + case 'backward': { + warning( + false, + '"%s" is not a valid value for revealOrder on . ' + + 'React uses the -s suffix in the spelling. Use "%ss" instead.', + revealOrder, + revealOrder.toLowerCase(), + ); + break; + } + default: + warning( + false, + '"%s" is not a supported revealOrder on . ' + + 'Did you mean "together", "forwards" or "backwards"?', + revealOrder, + ); + break; + } + } else { + warning( + false, + '%s is not a supported value for revealOrder on . ' + + 'Did you mean "together", "forwards" or "backwards"?', + revealOrder, + ); + } } } // We mark this as having captured but it really just says to the diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 930087c728..7fd9a6e40a 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -18,7 +18,10 @@ import type { ChildSet, } from './ReactFiberHostConfig'; import type {ReactEventComponentInstance} from 'shared/ReactTypes'; -import type {SuspenseState} from './ReactFiberSuspenseComponent'; +import type { + SuspenseState, + SuspenseListState, +} from './ReactFiberSuspenseComponent'; import type {SuspenseContext} from './ReactFiberSuspenseContext'; import { @@ -84,7 +87,12 @@ import { InvisibleParentSuspenseContext, hasSuspenseContext, popSuspenseContext, + pushSuspenseContext, + setShallowSuspenseContext, + ForceSuspenseFallback, + setDefaultShallowSuspenseContext, } from './ReactFiberSuspenseContext'; +import {isShowingAnyFallbacks} from './ReactFiberSuspenseComponent'; import { isContextProvider as isLegacyContextProvider, popContext as popLegacyContext, @@ -917,25 +925,80 @@ function completeWork( popSuspenseContext(workInProgress); if ((workInProgress.effectTag & DidCapture) === NoEffect) { - // This is the first pass. We need to figure out if anything is still - // suspended in the rendered set. - const renderedChildren = workInProgress.child; - // If new content unsuspended, but there's still some content that - // didn't. Then we need to do a second pass that forces everything - // to keep showing their fallbacks. - const needsRerender = hasSuspendedChildrenAndNewContent( - workInProgress, - renderedChildren, - ); - if (needsRerender) { - // Rerender the whole list, but this time, we'll force fallbacks - // to stay in place. - workInProgress.effectTag |= DidCapture; - // Reset the effect list before doing the second pass since that's now invalid. - workInProgress.firstEffect = workInProgress.lastEffect = null; - // Schedule work so we know not to bail out. - workInProgress.expirationTime = renderExpirationTime; - return workInProgress; + let suspenseListState: null | SuspenseListState = + workInProgress.memoizedState; + if ( + suspenseListState === null || + suspenseListState.rendering === null + ) { + // This is the first pass. We need to figure out if anything is still + // suspended in the rendered set. + const renderedChildren = workInProgress.child; + // If new content unsuspended, but there's still some content that + // didn't. Then we need to do a second pass that forces everything + // to keep showing their fallbacks. + const needsRerender = hasSuspendedChildrenAndNewContent( + workInProgress, + renderedChildren, + ); + if (needsRerender) { + // Rerender the whole list, but this time, we'll force fallbacks + // to stay in place. + workInProgress.effectTag |= DidCapture; + // Reset the effect list before doing the second pass since that's now invalid. + workInProgress.firstEffect = workInProgress.lastEffect = null; + // Schedule work so we know not to bail out. + workInProgress.expirationTime = renderExpirationTime; + return workInProgress; + } + } else { + // Append the rendered row to the child list. + let rendered = suspenseListState.rendering; + if (!suspenseListState.didSuspend) { + suspenseListState.didSuspend = isShowingAnyFallbacks(rendered); + } + if (suspenseListState.isBackwards) { + // The effect list of the backwards tail will have been added + // to the end. This breaks the guarantee that life-cycles fire in + // sibling order but that isn't a strong guarantee promised by React. + // Especially since these might also just pop in during future commits. + // Append to the beginning of the list. + rendered.sibling = workInProgress.child; + workInProgress.child = rendered; + } else { + let previousSibling = suspenseListState.last; + if (previousSibling !== null) { + previousSibling.sibling = rendered; + } else { + workInProgress.child = rendered; + } + suspenseListState.last = rendered; + } + } + + if (suspenseListState !== null && suspenseListState.tail !== null) { + // We still have tail rows to render. + // Pop a row. + let next = suspenseListState.tail; + suspenseListState.rendering = next; + suspenseListState.tail = next.sibling; + next.sibling = null; + + // Restore the context. + // TODO: We can probably just avoid popping it instead and only + // setting it the first time we go from not suspended to suspended. + let suspenseContext = suspenseStackCursor.current; + if (suspenseListState.didSuspend) { + suspenseContext = setShallowSuspenseContext( + suspenseContext, + ForceSuspenseFallback, + ); + } else { + suspenseContext = setDefaultShallowSuspenseContext(suspenseContext); + } + pushSuspenseContext(workInProgress, suspenseContext); + // Do a pass over the next row. + return next; } } else { workInProgress.effectTag &= ~DidCapture; diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js index ec385c0b95..187dcb1ad7 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js @@ -8,12 +8,20 @@ */ import type {Fiber} from './ReactFiber'; +import {SuspenseComponent} from 'shared/ReactWorkTags'; // TODO: This is now an empty object. Should we switch this to a boolean? export type SuspenseState = {||}; export type SuspenseListState = {| didSuspend: boolean, + isBackwards: boolean, + // The currently rendering tail row. + rendering: null | Fiber, + // The last of the already rendered children. + last: null | Fiber, + // Remaining rows on the tail of the list. + tail: null | Fiber, |}; export function shouldCaptureSuspense( @@ -43,3 +51,31 @@ export function shouldCaptureSuspense( // If the parent is not able to handle it, we must handle it. return true; } + +export function isShowingAnyFallbacks(row: Fiber): boolean { + let node = row; + while (node !== null) { + if (node.tag === SuspenseComponent) { + const state: SuspenseState | null = node.memoizedState; + if (state !== null) { + return true; + } + } else if (node.child !== null) { + node.child.return = node; + node = node.child; + continue; + } + if (node === row) { + return false; + } + while (node.sibling === null) { + if (node.return === null || node.return === row) { + return false; + } + node = node.return; + } + node.sibling.return = node.return; + node = node.sibling; + } + return false; +} diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js index 384c162d4c..2ca84312ad 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js @@ -56,7 +56,46 @@ describe('ReactSuspenseList', () => { expect(() => Scheduler.flushAll()).toWarnDev([ 'Warning: "something" is not a supported revealOrder on ' + - '. Did you mean "together"?' + + '. Did you mean "together", "forwards" or "backwards"?' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ]); + }); + + it('warns if a upper case revealOrder option is used', () => { + function Foo() { + return ( + + Content + + ); + } + + ReactNoop.render(); + + expect(() => Scheduler.flushAll()).toWarnDev([ + 'Warning: "TOGETHER" is not a valid value for revealOrder on ' + + '. Use lowercase "together" instead.' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ]); + }); + + it('warns if a misspelled revealOrder option is used', () => { + function Foo() { + return ( + + Content + + ); + } + + ReactNoop.render(); + + expect(() => Scheduler.flushAll()).toWarnDev([ + 'Warning: "forward" is not a valid value for revealOrder on ' + + '. React uses the -s suffix in the spelling. ' + + 'Use "forwards" instead.' + '\n in SuspenseList (at **)' + '\n in Foo (at **)', ]); @@ -562,4 +601,430 @@ describe('ReactSuspenseList', () => { , ); }); + + it('displays each items in "forwards" order', async () => { + let A = createAsyncText('A'); + let B = createAsyncText('B'); + let C = createAsyncText('C'); + + function Foo() { + return ( + + }> + + + }> + + + }> + + + + ); + } + + await C.resolve(); + + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + 'Loading A', + 'Loading B', + 'Loading C', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + + Loading A + Loading B + Loading C + , + ); + + await A.resolve(); + + expect(Scheduler).toFlushAndYield(['A', 'Suspend! [B]']); + + expect(ReactNoop).toMatchRenderedOutput( + + A + Loading B + Loading C + , + ); + + await B.resolve(); + + expect(Scheduler).toFlushAndYield(['B', 'C']); + + expect(ReactNoop).toMatchRenderedOutput( + + A + B + C + , + ); + }); + + it('displays each items in "backwards" order', async () => { + let A = createAsyncText('A'); + let B = createAsyncText('B'); + let C = createAsyncText('C'); + + function Foo() { + return ( + + }> + + + }> + + + }> + + + + ); + } + + await A.resolve(); + + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [C]', + 'Loading C', + 'Loading B', + 'Loading A', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + + Loading A + Loading B + Loading C + , + ); + + await C.resolve(); + + expect(Scheduler).toFlushAndYield(['C', 'Suspend! [B]']); + + expect(ReactNoop).toMatchRenderedOutput( + + Loading A + Loading B + C + , + ); + + await B.resolve(); + + expect(Scheduler).toFlushAndYield(['B', 'A']); + + expect(ReactNoop).toMatchRenderedOutput( + + A + B + C + , + ); + }); + + it('displays added row at the top "together" and the bottom in "forwards" order', async () => { + let A = createAsyncText('A'); + let B = createAsyncText('B'); + let C = createAsyncText('C'); + let D = createAsyncText('D'); + let E = createAsyncText('E'); + let F = createAsyncText('F'); + + function Foo({items}) { + return ( + + {items.map(([key, Component]) => ( + }> + + + ))} + + ); + } + + await B.resolve(); + await D.resolve(); + + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield(['B', 'D']); + + expect(ReactNoop).toMatchRenderedOutput( + + B + D + , + ); + + // Insert items in the beginning, middle and end. + ReactNoop.render( + , + ); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + 'Loading A', + 'B', + 'Suspend! [C]', + 'Loading C', + 'D', + 'Suspend! [E]', + 'Loading E', + 'Loading F', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + + Loading A + B + Loading C + D + Loading E + Loading F + , + ); + + await A.resolve(); + + expect(Scheduler).toFlushAndYield(['A', 'Suspend! [C]']); + + // Even though we could show A, it is still in a fallback state because + // C is not yet resolved. We need to resolve everything in the head first. + expect(ReactNoop).toMatchRenderedOutput( + + Loading A + B + Loading C + D + Loading E + Loading F + , + ); + + await C.resolve(); + + expect(Scheduler).toFlushAndYield(['A', 'C', 'Suspend! [E]']); + + // We can now resolve the full head. + expect(ReactNoop).toMatchRenderedOutput( + + A + B + C + D + Loading E + Loading F + , + ); + + await E.resolve(); + + expect(Scheduler).toFlushAndYield(['E', 'Suspend! [F]']); + + // In the tail we can resolve one-by-one. + expect(ReactNoop).toMatchRenderedOutput( + + A + B + C + D + E + Loading F + , + ); + + await F.resolve(); + + // We can also delete some items. + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield(['D', 'E', 'F']); + + expect(ReactNoop).toMatchRenderedOutput( + + D + E + F + , + ); + }); + + it('displays added row at the top "together" and the bottom in "forwards" order', async () => { + let A = createAsyncText('A'); + let B = createAsyncText('B'); + let D = createAsyncText('D'); + let F = createAsyncText('F'); + + function createSyncText(text) { + return function() { + return ; + }; + } + + let As = createSyncText('A'); + let Bs = createSyncText('B'); + let Cs = createSyncText('C'); + let Ds = createSyncText('D'); + let Es = createSyncText('E'); + let Fs = createSyncText('F'); + + function Foo({items}) { + return ( + + {items.map(([key, Component]) => ( + }> + + + ))} + + ); + } + + // The first pass doesn't suspend. + ReactNoop.render( + , + ); + expect(Scheduler).toFlushAndYield(['F', 'E', 'D', 'C', 'B', 'A']); + expect(ReactNoop).toMatchRenderedOutput( + + A + B + C + D + E + F + , + ); + + // Update items in the beginning, middle and end to start suspending. + ReactNoop.render( + , + ); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + 'Loading A', + 'Suspend! [B]', + 'Loading B', + 'C', + 'Suspend! [D]', + 'Loading D', + 'E', + 'Suspend! [F]', + 'Loading F', + ]); + + // This will suspend, since the boundaries are avoided. Give them + // time to display their loading states. + jest.advanceTimersByTime(500); + + expect(ReactNoop).toMatchRenderedOutput( + + + Loading A + + Loading B + C + + Loading D + E + + Loading F + , + ); + + await F.resolve(); + + expect(Scheduler).toFlushAndYield(['F']); + + // Even though we could show F, it is still in a fallback state because + // E is not yet resolved. We need to resolve everything in the head first. + expect(ReactNoop).toMatchRenderedOutput( + + + Loading A + + Loading B + C + + Loading D + E + + Loading F + , + ); + + await D.resolve(); + + expect(Scheduler).toFlushAndYield(['D', 'F', 'Suspend! [B]']); + + // We can now resolve the full head. + expect(ReactNoop).toMatchRenderedOutput( + + + Loading A + + Loading B + C + D + E + F + , + ); + + await B.resolve(); + + expect(Scheduler).toFlushAndYield(['B', 'Suspend! [A]']); + + // In the tail we can resolve one-by-one. + expect(ReactNoop).toMatchRenderedOutput( + + + Loading A + B + C + D + E + F + , + ); + + await A.resolve(); + + expect(Scheduler).toFlushAndYield(['A']); + + expect(ReactNoop).toMatchRenderedOutput( + + A + B + C + D + E + F + , + ); + }); });