From 50addf4c0e411e351de7290c8c60ec775c25c8c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 12 Aug 2019 15:58:38 -0700 Subject: [PATCH] Refactor Partial Hydration (#16346) * Move dehydrated to be child of regular SuspenseComponent We now store the comment node on SuspenseState instead and that indicates that this SuspenseComponent is still dehydrated. We also store a child but that is only used to represent the DOM node for deletions and getNextHostSibling. * Move logic from DehydratedSuspenseComponent to SuspenseComponent Forked based on SuspenseState.dehydrated instead. * Retry logic for dehydrated boundary We can now simplify the logic for retrying dehydrated boundaries without hydrating. This is becomes simply a reconciliation against the dehydrated fragment which gets deleted, and the new children gets inserted. * Remove dehydrated from throw Instead we use the regular Suspense path. To save code, we attach retry listeners in the commit phase even though technically we don't have to. * Pop to nearest Suspense I think this is right...? * Popping hydration state should skip past the dehydrated instance * Split mount from update and special case suspended second pass The DidCapture flag isn't used consistently in the same way. We need further refactor for this. * Reorganize update path If we remove the dehydration status in the first pass and then do a second pass because we suspended, then we need to continue as if it didn't previously suspend. Since there is no fragment child etc. However, we must readd the deletion. * Schedule context work on the boundary and not the child * Warn for Suspense hydration in legacy mode It does a two pass render that client renders the content. * Rename DehydratedSuspenseComponent -> DehydratedFragment This now doesn't represent a suspense boundary itself. Its parent does. This Fiber represents the fragment around the dehydrated content. * Refactor returns Avoids the temporary mutable variables. I kept losing track of them. * Add a comment explaining the type. Placing it in the type since that's the central point as opposed to spread out. --- ...DOMServerPartialHydration-test.internal.js | 77 +++- .../ReactDOMServerSuspense-test.internal.js | 20 +- .../src/ReactDebugFiberPerf.js | 4 +- packages/react-reconciler/src/ReactFiber.js | 10 + .../src/ReactFiberBeginWork.js | 376 +++++++++++------- .../src/ReactFiberCommitWork.js | 6 +- .../src/ReactFiberCompleteWork.js | 69 ++-- .../src/ReactFiberHydrationContext.js | 55 ++- .../src/ReactFiberNewContext.js | 21 +- .../src/ReactFiberSuspenseComponent.js | 21 +- .../react-reconciler/src/ReactFiberThrow.js | 40 +- .../src/ReactFiberUnwindWork.js | 36 +- .../src/ReactFiberWorkLoop.js | 4 - packages/shared/ReactWorkTags.js | 2 +- scripts/error-codes/codes.json | 4 +- 15 files changed, 453 insertions(+), 292 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 97642ad45a09f..1271e091fe859 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -90,6 +90,77 @@ describe('ReactDOMServerPartialHydration', () => { expect(ref.current).toBe(span); }); + it('warns and replaces the boundary content in legacy mode', async () => { + let suspend = false; + let resolve; + let promise = new Promise(resolvePromise => (resolve = resolvePromise)); + let ref = React.createRef(); + + function Child() { + if (suspend) { + throw promise; + } else { + return 'Hello'; + } + } + + function App() { + return ( +
+ + + + + +
+ ); + } + + // Don't suspend on the server. + suspend = false; + let finalHTML = ReactDOMServer.renderToString(); + + let container = document.createElement('div'); + container.innerHTML = finalHTML; + + let span = container.getElementsByTagName('span')[0]; + + // On the client we try to hydrate. + suspend = true; + expect(() => { + act(() => { + ReactDOM.hydrate(, container); + }); + }).toWarnDev( + 'Warning: Cannot hydrate Suspense in legacy mode. Switch from ' + + 'ReactDOM.hydrate(element, container) to ' + + 'ReactDOM.unstable_createSyncRoot(container, { hydrate: true })' + + '.render(element) or remove the Suspense components from the server ' + + 'rendered components.' + + '\n in Suspense (at **)' + + '\n in div (at **)' + + '\n in App (at **)', + ); + + // We're now in loading state. + expect(container.textContent).toBe('Loading...'); + + let span2 = container.getElementsByTagName('span')[0]; + // This is a new node. + expect(span).not.toBe(span2); + expect(ref.current).toBe(span2); + + // Resolving the promise should render the final content. + suspend = false; + resolve(); + await promise; + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + + // We should now have hydrated with a ref on the existing span. + expect(container.textContent).toBe('Hello'); + }); + it('can insert siblings before the dehydrated boundary', () => { let suspend = false; let promise = new Promise(() => {}); @@ -135,7 +206,8 @@ describe('ReactDOMServerPartialHydration', () => { suspend = true; act(() => { - ReactDOM.hydrate(, container); + let root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + root.render(); }); expect(container.firstChild.firstChild.tagName).not.toBe('DIV'); @@ -191,7 +263,8 @@ describe('ReactDOMServerPartialHydration', () => { // hydrating anyway. suspend = true; act(() => { - ReactDOM.hydrate(, container); + let root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + root.render(); }); expect(container.firstChild.children[1].textContent).toBe('Middle'); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js index 434fc6c8a507f..b5ecccca7e9df 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js @@ -37,7 +37,7 @@ function initModules() { }; } -const {resetModules, serverRender, itRenders} = ReactDOMServerIntegrationUtils( +const {resetModules, serverRender} = ReactDOMServerIntegrationUtils( initModules, ); @@ -102,8 +102,8 @@ describe('ReactDOMServerSuspense', () => { ); }); - itRenders('a SuspenseList component and its children', async render => { - const element = await render( + it('server renders a SuspenseList component and its children', async () => { + const example = (
A
@@ -111,8 +111,9 @@ describe('ReactDOMServerSuspense', () => {
B
 - , + ); + const element = await serverRender(example); const parent = element.parentNode; const divA = parent.children[0]; expect(divA.tagName).toBe('DIV'); @@ -120,5 +121,16 @@ describe('ReactDOMServerSuspense', () => { const divB = parent.children[1]; expect(divB.tagName).toBe('DIV'); expect(divB.textContent).toBe('B'); + + ReactTestUtils.act(() => { + const root = ReactDOM.unstable_createSyncRoot(parent, {hydrate: true}); + root.render(example); + }); + + const parent2 = element.parentNode; + const divA2 = parent2.children[0]; + const divB2 = parent2.children[1]; + expect(divA).toBe(divA2); + expect(divB).toBe(divB2); }); }); diff --git a/packages/react-reconciler/src/ReactDebugFiberPerf.js b/packages/react-reconciler/src/ReactDebugFiberPerf.js index 6abf5df279874..c08276945c2ec 100644 --- a/packages/react-reconciler/src/ReactDebugFiberPerf.js +++ b/packages/react-reconciler/src/ReactDebugFiberPerf.js @@ -21,7 +21,6 @@ import { ContextConsumer, Mode, SuspenseComponent, - DehydratedSuspenseComponent, } from 'shared/ReactWorkTags'; type MeasurementPhase = @@ -317,8 +316,7 @@ export function stopFailedWorkTimer(fiber: Fiber): void { } fiber._debugIsCurrentlyTiming = false; const warning = - fiber.tag === SuspenseComponent || - fiber.tag === DehydratedSuspenseComponent + fiber.tag === SuspenseComponent ? 'Rendering was suspended' : 'An error was thrown inside this error boundary'; endFiberMark(fiber, null, warning); diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 0463f0a9e889b..92b1455c1b329 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -24,6 +24,7 @@ import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {UpdateQueue} from './ReactUpdateQueue'; import type {ContextDependency} from './ReactFiberNewContext'; import type {HookType} from './ReactFiberHooks'; +import type {SuspenseInstance} from './ReactFiberHostConfig'; import invariant from 'shared/invariant'; import warningWithoutStack from 'shared/warningWithoutStack'; @@ -48,6 +49,7 @@ import { Profiler, SuspenseComponent, SuspenseListComponent, + DehydratedFragment, FunctionComponent, MemoComponent, SimpleMemoComponent, @@ -843,6 +845,14 @@ export function createFiberFromHostInstanceForDeletion(): Fiber { return fiber; } +export function createFiberFromDehydratedFragment( + dehydratedNode: SuspenseInstance, +): Fiber { + const fiber = createFiber(DehydratedFragment, null, null, NoMode); + fiber.stateNode = dehydratedNode; + return fiber; +} + export function createFiberFromPortal( portal: ReactPortal, mode: TypeOfMode, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 88fdf4c1ac417..e05fcfba34cbf 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -36,7 +36,6 @@ import { Profiler, SuspenseComponent, SuspenseListComponent, - DehydratedSuspenseComponent, MemoComponent, SimpleMemoComponent, LazyComponent, @@ -93,6 +92,7 @@ import {processUpdateQueue} from './ReactUpdateQueue'; import { NoWork, Never, + Sync, computeAsyncExpiration, } from './ReactFiberExpirationTime'; import { @@ -115,7 +115,6 @@ import {pushHostContext, pushHostContainer} from './ReactFiberHostContext'; import { suspenseStackCursor, pushSuspenseContext, - popSuspenseContext, InvisibleParentSuspenseContext, ForceSuspenseFallback, hasSuspenseContext, @@ -1475,8 +1474,9 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) { } } -// TODO: This is now an empty object. Should we just make it a boolean? -const SUSPENDED_MARKER: SuspenseState = ({}: any); +const SUSPENDED_MARKER: SuspenseState = { + dehydrated: null, +}; function shouldRemainOnFallback( suspenseContext: SuspenseContext, @@ -1511,21 +1511,23 @@ function updateSuspenseComponent( let suspenseContext: SuspenseContext = suspenseStackCursor.current; - let nextState = null; let nextDidTimeout = false; + const didSuspend = (workInProgress.effectTag & DidCapture) !== NoEffect; if ( - (workInProgress.effectTag & DidCapture) !== NoEffect || + didSuspend || shouldRemainOnFallback(suspenseContext, current, workInProgress) ) { // Something in this boundary's subtree already suspended. Switch to // rendering the fallback children. - nextState = SUSPENDED_MARKER; nextDidTimeout = true; workInProgress.effectTag &= ~DidCapture; } else { // Attempting the main content - if (current === null || current.memoizedState !== null) { + if ( + current === null || + (current.memoizedState: null | SuspenseState) !== null + ) { // This is a new mount or this boundary is already showing a fallback state. // Mark this subtree context as having at least one invisible parent that could // handle the fallback state. @@ -1582,29 +1584,24 @@ function updateSuspenseComponent( // custom reconciliation logic to preserve the state of the primary // children. It's essentially a very basic form of re-parenting. - // `child` points to the child fiber. In the normal case, this is the first - // fiber of the primary children set. In the timed-out case, it's a - // a fragment fiber containing the primary children. - let child; - // `next` points to the next fiber React should render. In the normal case, - // it's the same as `child`: the first fiber of the primary children set. - // In the timed-out case, it's a fragment fiber containing the *fallback* - // children -- we skip over the primary children entirely. - let next; if (current === null) { if (enableSuspenseServerRenderer) { // If we're currently hydrating, try to hydrate this boundary. // But only if this has a fallback. if (nextProps.fallback !== undefined) { tryToClaimNextHydratableInstance(workInProgress); - // This could've changed the tag if this was a dehydrated suspense component. - if (workInProgress.tag === DehydratedSuspenseComponent) { - popSuspenseContext(workInProgress); - return updateDehydratedSuspenseComponent( - null, - workInProgress, - renderExpirationTime, - ); + // This could've been a dehydrated suspense component. + const suspenseState: null | SuspenseState = + workInProgress.memoizedState; + if (suspenseState !== null) { + const dehydrated = suspenseState.dehydrated; + if (dehydrated !== null) { + return mountDehydratedSuspenseComponent( + workInProgress, + dehydrated, + renderExpirationTime, + ); + } } } } @@ -1646,26 +1643,121 @@ function updateSuspenseComponent( ); fallbackChildFragment.return = workInProgress; primaryChildFragment.sibling = fallbackChildFragment; - child = primaryChildFragment; // Skip the primary children, and continue working on the // fallback children. - next = fallbackChildFragment; + workInProgress.memoizedState = SUSPENDED_MARKER; + workInProgress.child = primaryChildFragment; + return fallbackChildFragment; } else { // Mount the primary children without an intermediate fragment fiber. const nextPrimaryChildren = nextProps.children; - child = next = mountChildFibers( + workInProgress.memoizedState = null; + return (workInProgress.child = mountChildFibers( workInProgress, null, nextPrimaryChildren, renderExpirationTime, - ); + )); } } else { // This is an update. This branch is more complicated because we need to // ensure the state of the primary children is preserved. - const prevState = current.memoizedState; - const prevDidTimeout = prevState !== null; - if (prevDidTimeout) { + const prevState: null | SuspenseState = current.memoizedState; + if (prevState !== null) { + if (enableSuspenseServerRenderer) { + const dehydrated = prevState.dehydrated; + if (dehydrated !== null) { + if (!didSuspend) { + return updateDehydratedSuspenseComponent( + current, + workInProgress, + dehydrated, + renderExpirationTime, + ); + } else if ( + (workInProgress.memoizedState: null | SuspenseState) !== null + ) { + // Something suspended and we should still be in dehydrated mode. + // Leave the existing child in place. + workInProgress.child = current.child; + // The dehydrated completion pass expects this flag to be there + // but the normal suspense pass doesn't. + workInProgress.effectTag |= DidCapture; + return null; + } else { + // Suspended but we should no longer be in dehydrated mode. + // Therefore we now have to render the fallback. Wrap the children + // in a fragment fiber to keep them separate from the fallback + // children. + const nextFallbackChildren = nextProps.fallback; + const primaryChildFragment = createFiberFromFragment( + // It shouldn't matter what the pending props are because we aren't + // going to render this fragment. + null, + mode, + NoWork, + null, + ); + primaryChildFragment.return = workInProgress; + + // This is always null since we never want the previous child + // that we're not going to hydrate. + primaryChildFragment.child = null; + + if ((workInProgress.mode & BatchedMode) === NoMode) { + // Outside of batched mode, we commit the effects from the + // partially completed, timed-out tree, too. + let progressedChild = (primaryChildFragment.child = + workInProgress.child); + while (progressedChild !== null) { + progressedChild.return = primaryChildFragment; + progressedChild = progressedChild.sibling; + } + } else { + // We will have dropped the effect list which contains the deletion. + // We need to reconcile to delete the current child. + reconcileChildFibers( + workInProgress, + current.child, + null, + renderExpirationTime, + ); + } + + // Because primaryChildFragment is a new fiber that we're inserting as the + // parent of a new tree, we need to set its treeBaseDuration. + if (enableProfilerTimer && workInProgress.mode & ProfileMode) { + // treeBaseDuration is the sum of all the child tree base durations. + let treeBaseDuration = 0; + let hiddenChild = primaryChildFragment.child; + while (hiddenChild !== null) { + treeBaseDuration += hiddenChild.treeBaseDuration; + hiddenChild = hiddenChild.sibling; + } + primaryChildFragment.treeBaseDuration = treeBaseDuration; + } + + // Create a fragment from the fallback children, too. + const fallbackChildFragment = createFiberFromFragment( + nextFallbackChildren, + mode, + renderExpirationTime, + null, + ); + fallbackChildFragment.return = workInProgress; + primaryChildFragment.sibling = fallbackChildFragment; + fallbackChildFragment.effectTag |= Placement; + primaryChildFragment.childExpirationTime = NoWork; + + workInProgress.memoizedState = SUSPENDED_MARKER; + workInProgress.child = primaryChildFragment; + + // Skip the primary children, and continue working on the + // fallback children. + return fallbackChildFragment; + } + } + } // The current tree already timed out. That means each child set is // wrapped in a fragment fiber. const currentPrimaryChildFragment: Fiber = (current.child: any); @@ -1721,11 +1813,12 @@ function updateSuspenseComponent( ); fallbackChildFragment.return = workInProgress; primaryChildFragment.sibling = fallbackChildFragment; - child = primaryChildFragment; primaryChildFragment.childExpirationTime = NoWork; // Skip the primary children, and continue working on the // fallback children. - next = fallbackChildFragment; + workInProgress.memoizedState = SUSPENDED_MARKER; + workInProgress.child = primaryChildFragment; + return fallbackChildFragment; } else { // No longer suspended. Switch back to showing the primary children, // and remove the intermediate fragment fiber. @@ -1745,7 +1838,8 @@ function updateSuspenseComponent( // the stateNode? // Continue rendering the children, like we normally do. - child = next = primaryChild; + workInProgress.memoizedState = null; + return (workInProgress.child = primaryChild); } } else { // The current tree has not already timed out. That means the primary @@ -1813,29 +1907,26 @@ function updateSuspenseComponent( fallbackChildFragment.return = workInProgress; primaryChildFragment.sibling = fallbackChildFragment; fallbackChildFragment.effectTag |= Placement; - child = primaryChildFragment; primaryChildFragment.childExpirationTime = NoWork; // Skip the primary children, and continue working on the // fallback children. - next = fallbackChildFragment; + workInProgress.memoizedState = SUSPENDED_MARKER; + workInProgress.child = primaryChildFragment; + return fallbackChildFragment; } else { // Still haven't timed out. Continue rendering the children, like we // normally do. + workInProgress.memoizedState = null; const nextPrimaryChildren = nextProps.children; - next = child = reconcileChildFibers( + return (workInProgress.child = reconcileChildFibers( workInProgress, currentPrimaryChild, nextPrimaryChildren, renderExpirationTime, - ); + )); } } - workInProgress.stateNode = current.stateNode; } - - workInProgress.memoizedState = nextState; - workInProgress.child = child; - return next; } function retrySuspenseComponentWithoutHydrating( @@ -1843,94 +1934,90 @@ function retrySuspenseComponentWithoutHydrating( workInProgress: Fiber, renderExpirationTime: ExpirationTime, ) { - // Detach from the current dehydrated boundary. - current.alternate = null; - workInProgress.alternate = null; - - // Insert a deletion in the effect list. - let returnFiber = workInProgress.return; - invariant( - returnFiber !== null, - 'Suspense boundaries are never on the root. ' + - 'This is probably a bug in React.', - ); - const last = returnFiber.lastEffect; - if (last !== null) { - last.nextEffect = current; - returnFiber.lastEffect = current; - } else { - returnFiber.firstEffect = returnFiber.lastEffect = current; - } - current.nextEffect = null; - current.effectTag = Deletion; - - popSuspenseContext(workInProgress); - - // Upgrade this work in progress to a real Suspense component. - workInProgress.tag = SuspenseComponent; - workInProgress.stateNode = null; + // We're now not suspended nor dehydrated. workInProgress.memoizedState = null; - // This is now an insertion. - workInProgress.effectTag |= Placement; - // Retry as a real Suspense component. - return updateSuspenseComponent(null, workInProgress, renderExpirationTime); + // Retry with the full children. + const nextProps = workInProgress.pendingProps; + const nextChildren = nextProps.children; + // This will ensure that the children get Placement effects and + // that the old child gets a Deletion effect. + // We could also call forceUnmountCurrentAndReconcile. + reconcileChildren( + current, + workInProgress, + nextChildren, + renderExpirationTime, + ); + return workInProgress.child; } -function updateDehydratedSuspenseComponent( - current: Fiber | null, +function mountDehydratedSuspenseComponent( workInProgress: Fiber, + suspenseInstance: SuspenseInstance, renderExpirationTime: ExpirationTime, -) { - pushSuspenseContext( - workInProgress, - setDefaultShallowSuspenseContext(suspenseStackCursor.current), - ); - const suspenseInstance = (workInProgress.stateNode: SuspenseInstance); - if (current === null) { - // During the first pass, we'll bail out and not drill into the children. - // Instead, we'll leave the content in place and try to hydrate it later. - if (isSuspenseInstanceFallback(suspenseInstance)) { - // This is a client-only boundary. Since we won't get any content from the server - // for this, we need to schedule that at a higher priority based on when it would - // have timed out. In theory we could render it in this pass but it would have the - // wrong priority associated with it and will prevent hydration of parent path. - // Instead, we'll leave work left on it to render it in a separate commit. - - // TODO This time should be the time at which the server rendered response that is - // a parent to this boundary was displayed. However, since we currently don't have - // a protocol to transfer that time, we'll just estimate it by using the current - // time. This will mean that Suspense timeouts are slightly shifted to later than - // they should be. - let serverDisplayTime = requestCurrentTime(); - // Schedule a normal pri update to render this content. - let newExpirationTime = computeAsyncExpiration(serverDisplayTime); - if (enableSchedulerTracing) { - markSpawnedWork(newExpirationTime); - } - workInProgress.expirationTime = newExpirationTime; - } else { - // We'll continue hydrating the rest at offscreen priority since we'll already - // be showing the right content coming from the server, it is no rush. - workInProgress.expirationTime = Never; - if (enableSchedulerTracing) { - markSpawnedWork(Never); - } +): null | Fiber { + // During the first pass, we'll bail out and not drill into the children. + // Instead, we'll leave the content in place and try to hydrate it later. + if ((workInProgress.mode & BatchedMode) === NoMode) { + if (__DEV__) { + warning( + false, + 'Cannot hydrate Suspense in legacy mode. Switch from ' + + 'ReactDOM.hydrate(element, container) to ' + + 'ReactDOM.unstable_createSyncRoot(container, { hydrate: true })' + + '.render(element) or remove the Suspense components from ' + + 'the server rendered components.', + ); + } + workInProgress.expirationTime = Sync; + } else if (isSuspenseInstanceFallback(suspenseInstance)) { + // This is a client-only boundary. Since we won't get any content from the server + // for this, we need to schedule that at a higher priority based on when it would + // have timed out. In theory we could render it in this pass but it would have the + // wrong priority associated with it and will prevent hydration of parent path. + // Instead, we'll leave work left on it to render it in a separate commit. + + // TODO This time should be the time at which the server rendered response that is + // a parent to this boundary was displayed. However, since we currently don't have + // a protocol to transfer that time, we'll just estimate it by using the current + // time. This will mean that Suspense timeouts are slightly shifted to later than + // they should be. + let serverDisplayTime = requestCurrentTime(); + // Schedule a normal pri update to render this content. + let newExpirationTime = computeAsyncExpiration(serverDisplayTime); + if (enableSchedulerTracing) { + markSpawnedWork(newExpirationTime); + } + workInProgress.expirationTime = newExpirationTime; + } else { + // We'll continue hydrating the rest at offscreen priority since we'll already + // be showing the right content coming from the server, it is no rush. + workInProgress.expirationTime = Never; + if (enableSchedulerTracing) { + markSpawnedWork(Never); } - - return null; - } - - if ((workInProgress.effectTag & DidCapture) !== NoEffect) { - // Something suspended. Leave the existing children in place. - // TODO: In non-concurrent mode, should we commit the nodes we have hydrated so far? - workInProgress.child = null; - return null; } + return null; +} +function updateDehydratedSuspenseComponent( + current: Fiber, + workInProgress: Fiber, + suspenseInstance: SuspenseInstance, + renderExpirationTime: ExpirationTime, +): null | Fiber { // We should never be hydrating at this point because it is the first pass, // but after we've already committed once. warnIfHydrating(); + if ((workInProgress.mode & BatchedMode) === NoMode) { + return retrySuspenseComponentWithoutHydrating( + current, + workInProgress, + renderExpirationTime, + ); + } + if (isSuspenseInstanceFallback(suspenseInstance)) { // This boundary is in a permanent fallback state. In this case, we'll never // get an update and we'll never be able to hydrate the final content. Let's just try the @@ -1967,8 +2054,8 @@ function updateDehydratedSuspenseComponent( // these should update this boundary to the permanent Fallback state instead. // Mark it as having captured (i.e. suspended). workInProgress.effectTag |= DidCapture; - // Leave the children in place. I.e. empty. - workInProgress.child = null; + // Leave the child in place. I.e. the dehydrated fragment. + workInProgress.child = current.child; // Register a callback to retry this boundary once the server has sent the result. registerSuspenseInstanceRetry( suspenseInstance, @@ -1977,7 +2064,10 @@ function updateDehydratedSuspenseComponent( return null; } else { // This is the first attempt. - reenterHydrationStateFromDehydratedSuspenseInstance(workInProgress); + reenterHydrationStateFromDehydratedSuspenseInstance( + workInProgress, + suspenseInstance, + ); const nextProps = workInProgress.pendingProps; const nextChildren = nextProps.children; workInProgress.child = mountChildFibers( @@ -2732,8 +2822,21 @@ function beginWork( break; case SuspenseComponent: { const state: SuspenseState | null = workInProgress.memoizedState; - const didTimeout = state !== null; - if (didTimeout) { + if (state !== null) { + if (enableSuspenseServerRenderer) { + if (state.dehydrated !== null) { + pushSuspenseContext( + workInProgress, + setDefaultShallowSuspenseContext(suspenseStackCursor.current), + ); + // We know that this component will suspend again because if it has + // been unsuspended it has committed as a resolved Suspense component. + // If it needs to be retried, it should have work scheduled on it. + workInProgress.effectTag |= DidCapture; + break; + } + } + // If this boundary is currently timed out, we need to decide // whether to retry the primary children, or to skip over it and // go straight to the fallback. Check the priority of the primary @@ -2780,19 +2883,6 @@ function beginWork( } break; } - case DehydratedSuspenseComponent: { - if (enableSuspenseServerRenderer) { - pushSuspenseContext( - workInProgress, - setDefaultShallowSuspenseContext(suspenseStackCursor.current), - ); - // We know that this component will suspend again because if it has - // been unsuspended it has committed as a regular Suspense component. - // If it needs to be retried, it should have work scheduled on it. - workInProgress.effectTag |= DidCapture; - } - break; - } case SuspenseListComponent: { const didSuspendBefore = (current.effectTag & DidCapture) !== NoEffect; @@ -3010,16 +3100,6 @@ function beginWork( renderExpirationTime, ); } - case DehydratedSuspenseComponent: { - if (enableSuspenseServerRenderer) { - return updateDehydratedSuspenseComponent( - current, - workInProgress, - renderExpirationTime, - ); - } - break; - } case SuspenseListComponent: { return updateSuspenseListComponent( current, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index e75ed5488e33b..3a6ff1f5aca05 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -43,7 +43,7 @@ import { HostPortal, Profiler, SuspenseComponent, - DehydratedSuspenseComponent, + DehydratedFragment, IncompleteClassComponent, MemoComponent, SimpleMemoComponent, @@ -967,7 +967,7 @@ function getHostSibling(fiber: Fiber): ?Instance { while ( node.tag !== HostComponent && node.tag !== HostText && - node.tag !== DehydratedSuspenseComponent + node.tag !== DehydratedFragment ) { // If it is not host node and, we might have a host node inside it. // Try to search down until we find one. @@ -1162,7 +1162,7 @@ function unmountHostComponents(current, renderPriorityLevel): void { } } else if ( enableSuspenseServerRenderer && - node.tag === DehydratedSuspenseComponent + node.tag === DehydratedFragment ) { // Delete the dehydrated suspense boundary and all of its content. if (currentParentIsContainer) { diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 4ca858158b4d8..6433a193ea69b 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -48,7 +48,6 @@ import { Profiler, SuspenseComponent, SuspenseListComponent, - DehydratedSuspenseComponent, MemoComponent, SimpleMemoComponent, LazyComponent, @@ -115,7 +114,6 @@ import {popProvider} from './ReactFiberNewContext'; import { prepareToHydrateHostInstance, prepareToHydrateHostTextInstance, - skipPastDehydratedSuspenseInstance, popHydrationState, resetHydrationState, } from './ReactFiberHydrationContext'; @@ -838,6 +836,38 @@ function completeWork( case SuspenseComponent: { popSuspenseContext(workInProgress); const nextState: null | SuspenseState = workInProgress.memoizedState; + + if (enableSuspenseServerRenderer) { + if (nextState !== null && nextState.dehydrated !== null) { + if (current === null) { + let wasHydrated = popHydrationState(workInProgress); + invariant( + wasHydrated, + 'A dehydrated suspense component was completed without a hydrated node. ' + + 'This is probably a bug in React.', + ); + if (enableSchedulerTracing) { + markSpawnedWork(Never); + } + return null; + } else { + // We should never have been in a hydration state if we didn't have a current. + // However, in some of those paths, we might have reentered a hydration state + // and then we might be inside a hydration state. In that case, we'll need to + // exit out of it. + resetHydrationState(); + if ((workInProgress.effectTag & DidCapture) === NoEffect) { + // This boundary did not suspend so it's now hydrated and unsuspended. + workInProgress.memoizedState = null; + } else { + // Something suspended. Schedule an effect to attach retry listeners. + workInProgress.effectTag |= Update; + } + return null; + } + } + } + if ((workInProgress.effectTag & DidCapture) !== NoEffect) { // Something suspended. Re-render with the fallback children. workInProgress.expirationTime = renderExpirationTime; @@ -849,8 +879,8 @@ function completeWork( let prevDidTimeout = false; if (current === null) { // In cases where we didn't find a suitable hydration boundary we never - // downgraded this to a DehydratedSuspenseComponent, but we still need to - // pop the hydration state since we might be inside the insertion tree. + // put this in dehydrated mode, but we still need to pop the hydration + // state since we might be inside the insertion tree. popHydrationState(workInProgress); } else { const prevState: null | SuspenseState = current.memoizedState; @@ -969,37 +999,6 @@ function completeWork( } break; } - case DehydratedSuspenseComponent: { - if (enableSuspenseServerRenderer) { - popSuspenseContext(workInProgress); - if (current === null) { - let wasHydrated = popHydrationState(workInProgress); - invariant( - wasHydrated, - 'A dehydrated suspense component was completed without a hydrated node. ' + - 'This is probably a bug in React.', - ); - skipPastDehydratedSuspenseInstance(workInProgress); - } else { - // We should never have been in a hydration state if we didn't have a current. - // However, in some of those paths, we might have reentered a hydration state - // and then we might be inside a hydration state. In that case, we'll need to - // exit out of it. - resetHydrationState(); - if ((workInProgress.effectTag & DidCapture) === NoEffect) { - // This boundary did not suspend so it's now hydrated. - // To handle any future suspense cases, we're going to now upgrade it - // to a Suspense component. We detach it from the existing current fiber. - current.alternate = null; - workInProgress.alternate = null; - workInProgress.tag = SuspenseComponent; - workInProgress.memoizedState = null; - workInProgress.stateNode = null; - } - } - } - break; - } case SuspenseListComponent: { popSuspenseContext(workInProgress); diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index 153503f0441c0..4b9cdf73d56b1 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -16,18 +16,21 @@ import type { Container, HostContext, } from './ReactFiberHostConfig'; +import type {SuspenseState} from './ReactFiberSuspenseComponent'; import { HostComponent, HostText, HostRoot, SuspenseComponent, - DehydratedSuspenseComponent, } from 'shared/ReactWorkTags'; import {Deletion, Placement} from 'shared/ReactSideEffectTags'; import invariant from 'shared/invariant'; -import {createFiberFromHostInstanceForDeletion} from './ReactFiber'; +import { + createFiberFromHostInstanceForDeletion, + createFiberFromDehydratedFragment, +} from './ReactFiber'; import { shouldSetTextContent, supportsHydration, @@ -82,12 +85,11 @@ function enterHydrationState(fiber: Fiber): boolean { function reenterHydrationStateFromDehydratedSuspenseInstance( fiber: Fiber, + suspenseInstance: SuspenseInstance, ): boolean { if (!supportsHydration) { return false; } - - const suspenseInstance = fiber.stateNode; nextHydratableInstance = getNextHydratableSibling(suspenseInstance); popToNextHostParent(fiber); isHydrating = true; @@ -221,11 +223,23 @@ function tryHydrate(fiber, nextInstance) { } case SuspenseComponent: { if (enableSuspenseServerRenderer) { - const suspenseInstance = canHydrateSuspenseInstance(nextInstance); + const suspenseInstance: null | SuspenseInstance = canHydrateSuspenseInstance( + nextInstance, + ); if (suspenseInstance !== null) { - // Downgrade the tag to a dehydrated component until we've hydrated it. - fiber.tag = DehydratedSuspenseComponent; - fiber.stateNode = (suspenseInstance: SuspenseInstance); + const suspenseState: SuspenseState = { + dehydrated: suspenseInstance, + }; + fiber.memoizedState = suspenseState; + // Store the dehydrated fragment as a child fiber. + // This simplifies the code for getHostSibling and deleting nodes, + // since it doesn't have to consider all Suspense boundaries and + // check if they're dehydrated ones or not. + const dehydratedFragment = createFiberFromDehydratedFragment( + suspenseInstance, + ); + dehydratedFragment.return = fiber; + fiber.child = dehydratedFragment; return true; } } @@ -354,7 +368,9 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { return shouldUpdate; } -function skipPastDehydratedSuspenseInstance(fiber: Fiber): void { +function skipPastDehydratedSuspenseInstance( + fiber: Fiber, +): null | HydratableInstance { if (!supportsHydration) { invariant( false, @@ -362,15 +378,15 @@ function skipPastDehydratedSuspenseInstance(fiber: Fiber): void { 'This error is likely caused by a bug in React. Please file an issue.', ); } - let suspenseInstance = fiber.stateNode; + let suspenseState: null | SuspenseState = fiber.memoizedState; + let suspenseInstance: null | SuspenseInstance = + suspenseState !== null ? suspenseState.dehydrated : null; invariant( suspenseInstance, 'Expected to have a hydrated suspense instance. ' + 'This error is likely caused by a bug in React. Please file an issue.', ); - nextHydratableInstance = getNextHydratableInstanceAfterSuspenseInstance( - suspenseInstance, - ); + return getNextHydratableInstanceAfterSuspenseInstance(suspenseInstance); } function popToNextHostParent(fiber: Fiber): void { @@ -379,7 +395,7 @@ function popToNextHostParent(fiber: Fiber): void { parent !== null && parent.tag !== HostComponent && parent.tag !== HostRoot && - parent.tag !== DehydratedSuspenseComponent + parent.tag !== SuspenseComponent ) { parent = parent.return; } @@ -425,9 +441,13 @@ function popHydrationState(fiber: Fiber): boolean { } popToNextHostParent(fiber); - nextHydratableInstance = hydrationParentFiber - ? getNextHydratableSibling(fiber.stateNode) - : null; + if (fiber.tag === SuspenseComponent) { + nextHydratableInstance = skipPastDehydratedSuspenseInstance(fiber); + } else { + nextHydratableInstance = hydrationParentFiber + ? getNextHydratableSibling(fiber.stateNode) + : null; + } return true; } @@ -449,6 +469,5 @@ export { tryToClaimNextHydratableInstance, prepareToHydrateHostInstance, prepareToHydrateHostTextInstance, - skipPastDehydratedSuspenseInstance, popHydrationState, }; diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index 35540c2ca8ca3..1839283ba6d55 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -25,7 +25,7 @@ import MAX_SIGNED_31_BIT_INT from './maxSigned31BitInt'; import { ContextProvider, ClassComponent, - DehydratedSuspenseComponent, + DehydratedFragment, } from 'shared/ReactWorkTags'; import invariant from 'shared/invariant'; @@ -249,15 +249,20 @@ export function propagateContextChange( nextFiber = fiber.type === workInProgress.type ? null : fiber.child; } else if ( enableSuspenseServerRenderer && - fiber.tag === DehydratedSuspenseComponent + fiber.tag === DehydratedFragment ) { - // If a dehydrated suspense component is in this subtree, we don't know + // If a dehydrated suspense bounudary is in this subtree, we don't know // if it will have any context consumers in it. The best we can do is - // mark it as having updates on its children. - if (fiber.expirationTime < renderExpirationTime) { - fiber.expirationTime = renderExpirationTime; + // mark it as having updates. + let parentSuspense = fiber.return; + invariant( + parentSuspense !== null, + 'We just came from a parent so we must have had a parent. This is a bug in React.', + ); + if (parentSuspense.expirationTime < renderExpirationTime) { + parentSuspense.expirationTime = renderExpirationTime; } - let alternate = fiber.alternate; + let alternate = parentSuspense.alternate; if ( alternate !== null && alternate.expirationTime < renderExpirationTime @@ -268,7 +273,7 @@ export function propagateContextChange( // because we want to schedule this fiber as having work // on its children. We'll use the childExpirationTime on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(fiber, renderExpirationTime); + scheduleWorkOnParentPath(parentSuspense, renderExpirationTime); nextFiber = fiber.sibling; } else { // Traverse down. diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js index 2440337b2e001..06f5aebc1b8de 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js @@ -8,12 +8,23 @@ */ import type {Fiber} from './ReactFiber'; +import type {SuspenseInstance} from './ReactFiberHostConfig'; 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 = {||}; +// A null SuspenseState represents an unsuspended normal Suspense boundary. +// A non-null SuspenseState means that it is blocked for one reason or another. +// - A non-null dehydrated field means it's blocked pending hydration. +// - A non-null dehydrated field can use isSuspenseInstancePending or +// isSuspenseInstanceFallback to query the reason for being dehydrated. +// - A null dehydrated field means it's blocked by something suspending and +// we're currently showing a fallback instead. +export type SuspenseState = {| + // If this boundary is still dehydrated, we store the SuspenseInstance + // here to indicate that it is dehydrated (flag) and for quick access + // to check things like isSuspenseInstancePending. + dehydrated: null | SuspenseInstance, +|}; export type SuspenseListTailMode = 'collapsed' | 'hidden' | void; @@ -42,6 +53,10 @@ export function shouldCaptureSuspense( // fallback. Otherwise, don't capture and bubble to the next boundary. const nextState: SuspenseState | null = workInProgress.memoizedState; if (nextState !== null) { + if (nextState.dehydrated !== null) { + // A dehydrated boundary always captures. + return true; + } return false; } const props = workInProgress.memoizedProps; diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 6f70e4e532d29..0e2d869b729a6 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -22,7 +22,6 @@ import { ClassComponent, HostRoot, SuspenseComponent, - DehydratedSuspenseComponent, IncompleteClassComponent, } from 'shared/ReactWorkTags'; import { @@ -32,10 +31,7 @@ import { ShouldCapture, LifecycleEffectMask, } from 'shared/ReactSideEffectTags'; -import { - enableSchedulerTracing, - enableSuspenseServerRenderer, -} from 'shared/ReactFeatureFlags'; +import {enableSchedulerTracing} from 'shared/ReactFeatureFlags'; import {NoMode, BatchedMode} from './ReactTypeOfMode'; import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent'; @@ -61,15 +57,11 @@ import { markLegacyErrorBoundaryAsFailed, isAlreadyFailedLegacyErrorBoundary, pingSuspendedRoot, - resolveRetryThenable, checkForWrongSuspensePriorityInDEV, } from './ReactFiberWorkLoop'; -import invariant from 'shared/invariant'; - import {Sync} from './ReactFiberExpirationTime'; -const PossiblyWeakSet = typeof WeakSet === 'function' ? WeakSet : Set; const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; function createRootErrorUpdate( @@ -323,36 +315,6 @@ function throwException( workInProgress.effectTag |= ShouldCapture; workInProgress.expirationTime = renderExpirationTime; - return; - } else if ( - enableSuspenseServerRenderer && - workInProgress.tag === DehydratedSuspenseComponent - ) { - attachPingListener(root, renderExpirationTime, thenable); - - // Since we already have a current fiber, we can eagerly add a retry listener. - let retryCache = workInProgress.memoizedState; - if (retryCache === null) { - retryCache = workInProgress.memoizedState = new PossiblyWeakSet(); - const current = workInProgress.alternate; - invariant( - current, - 'A dehydrated suspense boundary must commit before trying to render. ' + - 'This is probably a bug in React.', - ); - current.memoizedState = retryCache; - } - // Memoize using the boundary fiber to prevent redundant listeners. - if (!retryCache.has(thenable)) { - retryCache.add(thenable); - let retry = resolveRetryThenable.bind(null, workInProgress, thenable); - if (enableSchedulerTracing) { - retry = Schedule_tracing_wrap(retry); - } - thenable.then(retry, retry); - } - workInProgress.effectTag |= ShouldCapture; - workInProgress.expirationTime = renderExpirationTime; return; } // This boundary already captured during this render. Continue to the next diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index 425eb7d00ce3b..8ce3870468bed 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -9,6 +9,7 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; +import type {SuspenseState} from './ReactFiberSuspenseComponent'; import { ClassComponent, @@ -18,7 +19,6 @@ import { ContextProvider, SuspenseComponent, SuspenseListComponent, - DehydratedSuspenseComponent, } from 'shared/ReactWorkTags'; import {DidCapture, NoEffect, ShouldCapture} from 'shared/ReactSideEffectTags'; import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; @@ -71,6 +71,18 @@ function unwindWork( } case SuspenseComponent: { popSuspenseContext(workInProgress); + if (enableSuspenseServerRenderer) { + const suspenseState: null | SuspenseState = + workInProgress.memoizedState; + if (suspenseState !== null && suspenseState.dehydrated !== null) { + invariant( + workInProgress.alternate !== null, + 'Threw in newly mounted dehydrated component. This is likely a bug in ' + + 'React. Please file an issue.', + ); + resetHydrationState(); + } + } const effectTag = workInProgress.effectTag; if (effectTag & ShouldCapture) { workInProgress.effectTag = (effectTag & ~ShouldCapture) | DidCapture; @@ -79,23 +91,6 @@ function unwindWork( } return null; } - case DehydratedSuspenseComponent: { - if (enableSuspenseServerRenderer) { - popSuspenseContext(workInProgress); - if (workInProgress.alternate === null) { - // TODO: popHydrationState - } else { - resetHydrationState(); - } - const effectTag = workInProgress.effectTag; - if (effectTag & ShouldCapture) { - workInProgress.effectTag = (effectTag & ~ShouldCapture) | DidCapture; - // Captured a suspense effect. Re-render the boundary. - return workInProgress; - } - } - return null; - } case SuspenseListComponent: { popSuspenseContext(workInProgress); // SuspenseList doesn't actually catch anything. It should've been @@ -137,11 +132,6 @@ function unwindInterruptedWork(interruptedWork: Fiber) { case SuspenseComponent: popSuspenseContext(interruptedWork); break; - case DehydratedSuspenseComponent: - if (enableSuspenseServerRenderer) { - popSuspenseContext(interruptedWork); - } - break; case SuspenseListComponent: popSuspenseContext(interruptedWork); break; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index e67df799a20e3..a5a91b99dfce1 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -76,7 +76,6 @@ import { HostRoot, ClassComponent, SuspenseComponent, - DehydratedSuspenseComponent, FunctionComponent, ForwardRef, MemoComponent, @@ -2208,9 +2207,6 @@ export function resolveRetryThenable(boundaryFiber: Fiber, thenable: Thenable) { case SuspenseComponent: retryCache = boundaryFiber.stateNode; break; - case DehydratedSuspenseComponent: - retryCache = boundaryFiber.memoizedState; - break; default: invariant( false, diff --git a/packages/shared/ReactWorkTags.js b/packages/shared/ReactWorkTags.js index 5411e5b17be74..5c36488a09222 100644 --- a/packages/shared/ReactWorkTags.js +++ b/packages/shared/ReactWorkTags.js @@ -48,6 +48,6 @@ export const MemoComponent = 14; export const SimpleMemoComponent = 15; export const LazyComponent = 16; export const IncompleteClassComponent = 17; -export const DehydratedSuspenseComponent = 18; +export const DehydratedFragment = 18; export const SuspenseListComponent = 19; export const FundamentalComponent = 20; diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 604436b246022..392d643e4ef52 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -337,5 +337,7 @@ "336": "The \"%s\" event responder cannot be used via the \"useEvent\" hook.", "337": "An invalid event responder was provided to host component", "338": "ReactDOMServer does not yet support the fundamental API.", - "339": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponder()." + "339": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponder().", + "340": "Threw in newly mounted dehydrated component. This is likely a bug in React. Please file an issue.", + "341": "We just came from a parent so we must have had a parent. This is a bug in React." }