From 6f3c8332d8b2f92784a731e6cc6a707a92495a23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 7 Aug 2019 14:56:12 -0700 Subject: [PATCH] Reset hydration state after reentering (#16306) We might reenter a hydration state, when attempting to hydrate a boundary. We need to ensure that we reset it to not hydrating once we exit it. Otherwise the next sibling will still be in hydration mode. --- ...DOMServerPartialHydration-test.internal.js | 39 +++++++++++++++++++ .../src/ReactFiberBeginWork.js | 7 ++++ .../src/ReactFiberCompleteWork.js | 26 ++++++++----- .../src/ReactFiberHydrationContext.js | 11 ++++++ .../src/ReactFiberUnwindWork.js | 8 +++- 5 files changed, 80 insertions(+), 11 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 5b466f2431d96..e203baecea9bf 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -1057,4 +1057,43 @@ describe('ReactDOMServerPartialHydration', () => { expect(container.lastChild.nodeType).toBe(8); expect(container.lastChild.data).toBe('unrelated comment'); }); + + it('can hydrate TWO suspense boundaries', async () => { + let ref1 = React.createRef(); + let ref2 = React.createRef(); + + function App() { + return ( +
+ + 1 + + + 2 + +
+ ); + } + + // First we render the final HTML. With the streaming renderer + // this may have suspense points on the server but here we want + // to test the completed HTML. Don't suspend on the server. + let finalHTML = ReactDOMServer.renderToString(); + + let container = document.createElement('div'); + container.innerHTML = finalHTML; + + let span1 = container.getElementsByTagName('span')[0]; + let span2 = container.getElementsByTagName('span')[1]; + + // On the client we don't have all data yet but we want to start + // hydrating anyway. + let root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + root.render(); + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + + expect(ref1.current).toBe(span1); + expect(ref2.current).toBe(span2); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index f0f22b38384c2..c4ab486ada5cb 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -148,6 +148,7 @@ import { reenterHydrationStateFromDehydratedSuspenseInstance, resetHydrationState, tryToClaimNextHydratableInstance, + warnIfHydrating, } from './ReactFiberHydrationContext'; import { adoptClassInstance, @@ -1910,12 +1911,18 @@ function updateDehydratedSuspenseComponent( } 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; } + + // We should never be hydrating at this point because it is the first pass, + // but after we've already committed once. + warnIfHydrating(); + 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 diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index aee20f6fc67b2..c8fe383451794 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -117,6 +117,7 @@ import { prepareToHydrateHostTextInstance, skipPastDehydratedSuspenseInstance, popHydrationState, + resetHydrationState, } from './ReactFiberHydrationContext'; import { enableSchedulerTracing, @@ -982,15 +983,22 @@ function completeWork( markSpawnedWork(Never); } skipPastDehydratedSuspenseInstance(workInProgress); - } else 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; + } 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; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.js b/packages/react-reconciler/src/ReactFiberHydrationContext.js index 07647d523c4e3..153503f0441c0 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.js @@ -51,6 +51,7 @@ import { didNotFindHydratableSuspenseInstance, } from './ReactFiberHostConfig'; import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; +import warning from 'shared/warning'; // The deepest Fiber on the stack involved in a hydration context. // This may have been an insertion or a hydration. @@ -58,6 +59,15 @@ let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; +function warnIfHydrating() { + if (__DEV__) { + warning( + !isHydrating, + 'We should not be hydrating here. This is a bug in React. Please file a bug.', + ); + } +} + function enterHydrationState(fiber: Fiber): boolean { if (!supportsHydration) { return false; @@ -432,6 +442,7 @@ function resetHydrationState(): void { } export { + warnIfHydrating, enterHydrationState, reenterHydrationStateFromDehydratedSuspenseInstance, resetHydrationState, diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index 3db10c23acc92..425eb7d00ce3b 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -25,6 +25,7 @@ import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; import {popHostContainer, popHostContext} from './ReactFiberHostContext'; import {popSuspenseContext} from './ReactFiberSuspenseContext'; +import {resetHydrationState} from './ReactFiberHydrationContext'; import { isContextProvider as isLegacyContextProvider, popContext as popLegacyContext, @@ -80,8 +81,12 @@ function unwindWork( } case DehydratedSuspenseComponent: { if (enableSuspenseServerRenderer) { - // TODO: popHydrationState popSuspenseContext(workInProgress); + if (workInProgress.alternate === null) { + // TODO: popHydrationState + } else { + resetHydrationState(); + } const effectTag = workInProgress.effectTag; if (effectTag & ShouldCapture) { workInProgress.effectTag = (effectTag & ~ShouldCapture) | DidCapture; @@ -134,7 +139,6 @@ function unwindInterruptedWork(interruptedWork: Fiber) { break; case DehydratedSuspenseComponent: if (enableSuspenseServerRenderer) { - // TODO: popHydrationState popSuspenseContext(interruptedWork); } break;