diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 4821f4205e..73a3c4c179 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -27,7 +27,7 @@ import type {ReactEventComponentInstance} from 'shared/ReactTypes'; import invariant from 'shared/invariant'; import warningWithoutStack from 'shared/warningWithoutStack'; import {enableProfilerTimer, enableFlareAPI} from 'shared/ReactFeatureFlags'; -import {NoEffect} from 'shared/ReactSideEffectTags'; +import {NoEffect, Placement} from 'shared/ReactSideEffectTags'; import {ConcurrentRoot, BatchedRoot} from 'shared/ReactRootTags'; import { IndeterminateComponent, @@ -494,8 +494,9 @@ export function resetWorkInProgress( // We assume pendingProps, index, key, ref, return are still untouched to // avoid doing another reconciliation. - // Reset the effect tag. - workInProgress.effectTag = NoEffect; + // Reset the effect tag but keep any Placement tags, since that's something + // that child fiber is setting, not the reconciliation. + workInProgress.effectTag &= Placement; // The effect list is no longer valid. workInProgress.nextEffect = null; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 0bf2484775..3c62caf099 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -125,7 +125,7 @@ import { addSubtreeSuspenseContext, setShallowSuspenseContext, } from './ReactFiberSuspenseContext'; -import {isShowingAnyFallbacks} from './ReactFiberSuspenseComponent'; +import {findFirstSuspended} from './ReactFiberSuspenseComponent'; import { pushProvider, propagateContextChange, @@ -2000,7 +2000,7 @@ function findLastContentRow(firstChild: null | Fiber): null | Fiber { while (row !== null) { let currentRow = row.alternate; // New rows can't be content rows. - if (currentRow !== null && !isShowingAnyFallbacks(currentRow)) { + if (currentRow !== null && findFirstSuspended(currentRow) === null) { lastContentRow = row; } row = row.sibling; @@ -2072,12 +2072,12 @@ function validateTailOptions( ) { if (__DEV__) { if (tailMode !== undefined && !didWarnAboutTailOptions[tailMode]) { - if (tailMode !== 'collapsed') { + if (tailMode !== 'collapsed' && tailMode !== 'hidden') { didWarnAboutTailOptions[tailMode] = true; warning( false, '"%s" is not a supported value for tail on . ' + - 'Did you mean "collapsed"?', + 'Did you mean "collapsed" or "hidden"?', tailMode, ); } else if (revealOrder !== 'forwards' && revealOrder !== 'backwards') { @@ -2242,10 +2242,10 @@ function updateSuspenseListComponent( pushSuspenseContext(workInProgress, suspenseContext); if ((workInProgress.mode & BatchedMode) === NoMode) { - workInProgress.memoizedState = null; - } else { // Outside of batched mode, SuspenseList doesn't work so we just // use make it a noop by treating it as the default revealOrder. + workInProgress.memoizedState = null; + } else { switch (revealOrder) { case 'forwards': { let lastContentRow = findLastContentRow(workInProgress.child); @@ -2281,7 +2281,7 @@ function updateSuspenseListComponent( while (row !== null) { let currentRow = row.alternate; // New rows can't be content rows. - if (currentRow !== null && !isShowingAnyFallbacks(currentRow)) { + if (currentRow !== null && findFirstSuspended(currentRow) === null) { // This is the beginning of the main content. workInProgress.child = row; break; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 22df199a47..faa6548120 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -92,7 +92,7 @@ import { ForceSuspenseFallback, setDefaultShallowSuspenseContext, } from './ReactFiberSuspenseContext'; -import {isShowingAnyFallbacks} from './ReactFiberSuspenseComponent'; +import {findFirstSuspended} from './ReactFiberSuspenseComponent'; import { isContextProvider as isLegacyContextProvider, popContext as popLegacyContext, @@ -537,6 +537,32 @@ function cutOffTailIfNeeded( hasRenderedATailFallback: boolean, ) { switch (renderState.tailMode) { + case 'hidden': { + // Any insertions at the end of the tail list after this point + // should be invisible. If there are already mounted boundaries + // anything before them are not considered for collapsing. + // Therefore we need to go through the whole tail to find if + // there are any. + let tailNode = renderState.tail; + let lastTailNode = null; + while (tailNode !== null) { + if (tailNode.alternate !== null) { + lastTailNode = tailNode; + } + tailNode = tailNode.sibling; + } + // Next we're simply going to delete all insertions after the + // last rendered item. + if (lastTailNode === null) { + // All remaining items in the tail are insertions. + renderState.tail = null; + } else { + // Detach the insertion after the last node that was already + // inserted. + lastTailNode.sibling = null; + } + break; + } case 'collapsed': { // Any insertions at the end of the tail list after this point // should be invisible. If there are already mounted boundaries @@ -572,89 +598,6 @@ function cutOffTailIfNeeded( } } -// Note this, might mutate the workInProgress passed in. -function hasSuspendedChildrenAndNewContent( - workInProgress: Fiber, - firstChild: null | Fiber, -): boolean { - // Traversal to see if any of the immediately nested Suspense boundaries - // are in their fallback states. I.e. something suspended in them. - // And if some of them have new content that wasn't already visible. - let hasSuspendedBoundaries = false; - let hasNewContent = false; - - let node = firstChild; - while (node !== null) { - // TODO: Hidden subtrees should not be considered. - if (node.tag === SuspenseComponent) { - const state: SuspenseState | null = node.memoizedState; - const isShowingFallback = state !== null; - if (isShowingFallback) { - // Tag the parent fiber as having suspended boundaries. - if (!hasSuspendedBoundaries) { - workInProgress.effectTag |= DidCapture; - } - - hasSuspendedBoundaries = true; - - if (node.updateQueue !== null) { - // If this is a newly suspended tree, it might not get committed as - // part of the second pass. In that case nothing will subscribe to - // its thennables. Instead, we'll transfer its thennables to the - // SuspenseList so that it can retry if they resolve. - // There might be multiple of these in the list but since we're - // going to wait for all of them anyway, it doesn't really matter - // which ones gets to ping. In theory we could get clever and keep - // track of how many dependencies remain but it gets tricky because - // in the meantime, we can add/remove/change items and dependencies. - // We might bail out of the loop before finding any but that - // doesn't matter since that means that the other boundaries that - // we did find already has their listeners attached. - workInProgress.updateQueue = node.updateQueue; - workInProgress.effectTag |= Update; - } - } else { - const current = node.alternate; - const wasNotShowingContent = - current === null || current.memoizedState !== null; - if (wasNotShowingContent) { - hasNewContent = true; - } - } - if (hasSuspendedBoundaries && hasNewContent) { - return true; - } - } else { - // TODO: We can probably just use the information from the list and not - // drill into its children just like if it was a Suspense boundary. - if (node.tag === SuspenseListComponent && node.updateQueue !== null) { - // If there's a nested SuspenseList, we might have transferred - // the thennables set to it already so we must get it from there. - workInProgress.updateQueue = node.updateQueue; - workInProgress.effectTag |= Update; - } - - if (node.child !== null) { - node.child.return = node; - node = node.child; - continue; - } - } - if (node === workInProgress) { - return false; - } - while (node.sibling === null) { - if (node.return === null || node.return === workInProgress) { - return false; - } - node = node.return; - } - node.sibling.return = node.return; - node = node.sibling; - } - return false; -} - function completeWork( current: Fiber | null, workInProgress: Fiber, @@ -988,7 +931,7 @@ function completeWork( if (!didSuspendAlready) { // 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. @@ -996,47 +939,93 @@ function completeWork( // We might be suspended if something in this render pass suspended, or // something in the previous committed pass suspended. Otherwise, // there's no chance so we can skip the expensive call to - // hasSuspendedChildrenAndNewContent. + // findFirstSuspended. let cannotBeSuspended = renderHasNotSuspendedYet() && (current === null || (current.effectTag & DidCapture) === NoEffect); - let needsRerender = - !cannotBeSuspended && - hasSuspendedChildrenAndNewContent(workInProgress, renderedChildren); - if (needsRerender) { - // Rerender the whole list, but this time, we'll force fallbacks - // to stay in place. - // Reset the effect list before doing the second pass since that's now invalid. - workInProgress.firstEffect = workInProgress.lastEffect = null; - // Reset the child fibers to their original state. - resetChildFibers(workInProgress, renderExpirationTime); + if (!cannotBeSuspended) { + let row = workInProgress.child; + while (row !== null) { + let suspended = findFirstSuspended(row); + if (suspended !== null) { + didSuspendAlready = true; + workInProgress.effectTag |= DidCapture; + cutOffTailIfNeeded(renderState, false); - // Set up the Suspense Context to force suspense and immediately - // rerender the children. - pushSuspenseContext( - workInProgress, - setShallowSuspenseContext( - suspenseStackCursor.current, - ForceSuspenseFallback, - ), - ); - return workInProgress.child; + // If this is a newly suspended tree, it might not get committed as + // part of the second pass. In that case nothing will subscribe to + // its thennables. Instead, we'll transfer its thennables to the + // SuspenseList so that it can retry if they resolve. + // There might be multiple of these in the list but since we're + // going to wait for all of them anyway, it doesn't really matter + // which ones gets to ping. In theory we could get clever and keep + // track of how many dependencies remain but it gets tricky because + // in the meantime, we can add/remove/change items and dependencies. + // We might bail out of the loop before finding any but that + // doesn't matter since that means that the other boundaries that + // we did find already has their listeners attached. + let newThennables = suspended.updateQueue; + if (newThennables !== null) { + workInProgress.updateQueue = newThennables; + workInProgress.effectTag |= Update; + } + + // Rerender the whole list, but this time, we'll force fallbacks + // to stay in place. + // Reset the effect list before doing the second pass since that's now invalid. + workInProgress.firstEffect = workInProgress.lastEffect = null; + // Reset the child fibers to their original state. + resetChildFibers(workInProgress, renderExpirationTime); + + // Set up the Suspense Context to force suspense and immediately + // rerender the children. + pushSuspenseContext( + workInProgress, + setShallowSuspenseContext( + suspenseStackCursor.current, + ForceSuspenseFallback, + ), + ); + return workInProgress.child; + } + row = row.sibling; + } } - // hasSuspendedChildrenAndNewContent could've set didSuspendAlready - didSuspendAlready = - (workInProgress.effectTag & DidCapture) !== NoEffect; - } - if (didSuspendAlready) { + } else { cutOffTailIfNeeded(renderState, false); } // Next we're going to render the tail. } else { // Append the rendered row to the child list. if (!didSuspendAlready) { - if (isShowingAnyFallbacks(renderedTail)) { + let suspended = findFirstSuspended(renderedTail); + if (suspended !== null) { workInProgress.effectTag |= DidCapture; didSuspendAlready = true; cutOffTailIfNeeded(renderState, true); + // This might have been modified. + if ( + renderState.tail === null && + renderState.tailMode === 'hidden' + ) { + // We need to delete the row we just rendered. + // Ensure we transfer the update queue to the parent. + let newThennables = suspended.updateQueue; + if (newThennables !== null) { + workInProgress.updateQueue = newThennables; + workInProgress.effectTag |= Update; + } + // Reset the effect list to what it w as before we rendered this + // child. The nested children have already appended themselves. + let lastEffect = (workInProgress.lastEffect = + renderState.lastEffect); + // Remove any effects that were appended after this point. + if (lastEffect !== null) { + lastEffect.nextEffect = null; + } + // We're done. + return null; + } } else if ( now() > renderState.tailExpiration && renderExpirationTime > Never @@ -1093,6 +1082,7 @@ function completeWork( let next = renderState.tail; renderState.rendering = next; renderState.tail = next.sibling; + renderState.lastEffect = workInProgress.lastEffect; next.sibling = null; // Restore the context. diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js index 317cbf20a3..2440337b2e 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js @@ -8,13 +8,14 @@ */ import type {Fiber} from './ReactFiber'; -import {SuspenseComponent} from 'shared/ReactWorkTags'; +import {SuspenseComponent, SuspenseListComponent} from 'shared/ReactWorkTags'; +import {NoEffect, DidCapture} from 'shared/ReactSideEffectTags'; // TODO: This is now an empty object. Should we switch this to a boolean? // Alternatively we can make this use an effect tag similar to SuspenseList. export type SuspenseState = {||}; -export type SuspenseListTailMode = 'collapsed' | void; +export type SuspenseListTailMode = 'collapsed' | 'hidden' | void; export type SuspenseListRenderState = {| isBackwards: boolean, @@ -28,6 +29,9 @@ export type SuspenseListRenderState = {| tailExpiration: number, // Tail insertions setting. tailMode: SuspenseListTailMode, + // Last Effect before we rendered the "rendering" item. + // Used to remove new effects added by the rendered item. + lastEffect: null | Fiber, |}; export function shouldCaptureSuspense( @@ -58,13 +62,23 @@ export function shouldCaptureSuspense( return true; } -export function isShowingAnyFallbacks(row: Fiber): boolean { +export function findFirstSuspended(row: Fiber): null | Fiber { let node = row; while (node !== null) { if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - return true; + return node; + } + } else if ( + node.tag === SuspenseListComponent && + // revealOrder undefined can't be trusted because it don't + // keep track of whether it suspended or not. + node.memoizedProps.revealOrder !== undefined + ) { + let didSuspend = (node.effectTag & DidCapture) !== NoEffect; + if (didSuspend) { + return node; } } else if (node.child !== null) { node.child.return = node; @@ -72,16 +86,16 @@ export function isShowingAnyFallbacks(row: Fiber): boolean { continue; } if (node === row) { - return false; + return null; } while (node.sibling === null) { if (node.return === null || node.return === row) { - return false; + return null; } node = node.return; } node.sibling.return = node.return; node = node.sibling; } - return false; + return null; } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js index d8226c32e5..94fc54601c 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js @@ -640,6 +640,9 @@ describe('ReactSuspenseList', () => { 'Loading B', 'Suspend! [C]', 'Loading C', + 'A', + 'Loading B', + 'Loading C', ]); // This will suspend, since the boundaries are avoided. Give them @@ -859,6 +862,10 @@ describe('ReactSuspenseList', () => { 'Suspend! [C]', 'Loading C', 'D', + 'Loading A', + 'B', + 'Loading C', + 'D', 'Loading E', 'Loading F', ]); @@ -1013,6 +1020,16 @@ describe('ReactSuspenseList', () => { 'E', 'Suspend! [F]', 'Loading F', + '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 @@ -1251,7 +1268,7 @@ describe('ReactSuspenseList', () => { expect(() => Scheduler.unstable_flushAll()).toWarnDev([ 'Warning: "collapse" is not a supported value for tail on ' + - '. Did you mean "collapsed"?' + + '. Did you mean "collapsed" or "hidden"?' + '\n in SuspenseList (at **)' + '\n in Foo (at **)', ]); @@ -1392,6 +1409,10 @@ describe('ReactSuspenseList', () => { 'Suspend! [C]', 'Loading C', 'D', + 'A', + 'Loading B', + 'Loading C', + 'D', 'Loading E', ]); @@ -1516,6 +1537,10 @@ describe('ReactSuspenseList', () => { 'Suspend! [E]', 'Loading E', 'F', + 'C', + 'Loading D', + 'Loading E', + 'F', 'Loading B', ]); @@ -1530,15 +1555,15 @@ describe('ReactSuspenseList', () => { , ); - await E.resolve(); + await D.resolve(); - expect(Scheduler).toFlushAndYield(['Suspend! [D]', 'E']); + expect(Scheduler).toFlushAndYield(['D', 'Suspend! [E]']); // Incremental loading is suspended. jest.advanceTimersByTime(500); - // Even though E is unsuspended, it's still in loading state because - // it is blocked by D. + // Even though D is unsuspended, it's still in loading state because + // it is blocked by E. expect(ReactNoop).toMatchRenderedOutput( Loading B @@ -1650,6 +1675,11 @@ describe('ReactSuspenseList', () => { 'Loading C', 'Suspend! [D]', 'Loading D', + 'A', + 'Loading B', + 'Loading C', + 'Suspend! [D]', + 'Loading D', 'Loading E', ]); @@ -1733,4 +1763,67 @@ describe('ReactSuspenseList', () => { , ); }); + + it('only shows no initial loading state "hidden" tail insertions', async () => { + let A = createAsyncText('A'); + let B = createAsyncText('B'); + let C = createAsyncText('C'); + + function Foo() { + return ( + + }> + + + }> + + + }> + + + + ); + } + + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading A']); + + expect(ReactNoop).toMatchRenderedOutput(null); + + await A.resolve(); + + expect(Scheduler).toFlushAndYield(['A', 'Suspend! [B]', 'Loading B']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); + + expect(ReactNoop).toMatchRenderedOutput(A); + + await B.resolve(); + + expect(Scheduler).toFlushAndYield(['B', 'Suspend! [C]', 'Loading C']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); + + expect(ReactNoop).toMatchRenderedOutput( + + A + B + , + ); + + await C.resolve(); + + expect(Scheduler).toFlushAndYield(['C']); + + expect(ReactNoop).toMatchRenderedOutput( + + A + B + C + , + ); + }); });