From c36d7784c6bb042014ce84caa4e861fdbcb674f1 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 10:23:08 -0400 Subject: [PATCH 01/14] Throw on hydration mismatch --- ...DOMServerPartialHydration-test.internal.js | 92 +++++- .../src/ReactFiberHydrationContext.new.js | 9 + .../src/ReactFiberHydrationContext.old.js | 9 + .../src/ReactFiberThrow.new.js | 280 +++++++++--------- .../src/ReactFiberThrow.old.js | 280 +++++++++--------- 5 files changed, 398 insertions(+), 272 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 505a100df3aa6..fd0d329a1c590 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -197,12 +197,7 @@ describe('ReactDOMServerPartialHydration', () => { // hydrating anyway. suspend = true; ReactDOM.hydrateRoot(container, ); - expect(() => { - Scheduler.unstable_flushAll(); - }).toErrorDev( - // TODO: This error should not be logged in this case. It's a false positive. - 'Did not expect server HTML to contain the text node "Hello" in
.', - ); + Scheduler.unstable_flushAll(); jest.runAllTimers(); // Expect the server-generated HTML to stay intact. @@ -218,6 +213,91 @@ describe('ReactDOMServerPartialHydration', () => { expect(container.textContent).toBe('HelloHello'); }); + it('falls back to client rendering boundary on mismatch', async () => { + let client = false; + let suspend = false; + let resolve; + const promise = new Promise(resolvePromise => { + resolve = () => { + suspend = false; + resolvePromise(); + }; + }); + function Child() { + if (suspend) { + Scheduler.unstable_yieldValue('Suspend'); + throw promise; + } else { + Scheduler.unstable_yieldValue('Hello'); + return 'Hello'; + } + } + function Component({shouldMismatch}) { + Scheduler.unstable_yieldValue('Component'); + if (shouldMismatch && client) { + return
Mismatch
; + } + return
Component
; + } + function App() { + return ( + + + + + + + + ); + } + const finalHTML = ReactDOMServer.renderToString(); + const container = document.createElement('div'); + container.innerHTML = finalHTML; + expect(Scheduler).toHaveYielded([ + 'Hello', + 'Component', + 'Component', + 'Component', + 'Component', + ]); + + suspend = true; + client = true; + + ReactDOM.hydrateRoot(container, ); + expect(Scheduler).toFlushAndYield([ + 'Suspend', + 'Component', + 'Component', + 'Component', + 'Component', + ]); + jest.runAllTimers(); + + suspend = false; + resolve(); + await promise; + + debugger; + expect(Scheduler).toFlushAndYield([ + // first pass, mismatches at end + 'Hello', + 'Component', + 'Component', + 'Component', + 'Component', + // second pass as client render + 'Hello', + 'Component', + 'Component', + 'Component', + 'Component', + ]); + expect(container.innerHTML).toBe( + 'Hello
Component
Component
Component
Mismatch
', + ); + }); + it('calls the hydration callbacks after hydration or deletion', async () => { let suspend = false; let resolve; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 7275f1663cad8..6a342870ebb33 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -8,6 +8,7 @@ */ import type {Fiber} from './ReactInternalTypes'; +import {NoMode, ConcurrentMode} from './ReactTypeOfMode'; import type { Instance, TextInstance, @@ -313,12 +314,19 @@ function tryHydrate(fiber, nextInstance) { } } +function throwOnHydrationMismatchIfConcurrentMode(fiber) { + if ((fiber.mode & ConcurrentMode) !== NoMode) { + throw new Error('Hydration mismatch'); + } +} + function tryToClaimNextHydratableInstance(fiber: Fiber): void { if (!isHydrating) { return; } let nextInstance = nextHydratableInstance; if (!nextInstance) { + throwOnHydrationMismatchIfConcurrentMode(fiber); // Nothing to hydrate. Make it an insertion. insertNonHydratedInstance((hydrationParentFiber: any), fiber); isHydrating = false; @@ -327,6 +335,7 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void { } const firstAttemptedInstance = nextInstance; if (!tryHydrate(fiber, nextInstance)) { + throwOnHydrationMismatchIfConcurrentMode(fiber); // If we can't hydrate this instance let's try the next one. // We use this as a heuristic. It's based on intuition and not data so it // might be flawed or unnecessary. diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index 654de3f9a2894..95a8ea0f29d1e 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -8,6 +8,7 @@ */ import type {Fiber} from './ReactInternalTypes'; +import {NoMode, ConcurrentMode} from './ReactTypeOfMode'; import type { Instance, TextInstance, @@ -313,12 +314,19 @@ function tryHydrate(fiber, nextInstance) { } } +function throwOnHydrationMismatchIfConcurrentMode(fiber) { + if ((fiber.mode & ConcurrentMode) !== NoMode) { + throw new Error('Hydration mismatch'); + } +} + function tryToClaimNextHydratableInstance(fiber: Fiber): void { if (!isHydrating) { return; } let nextInstance = nextHydratableInstance; if (!nextInstance) { + throwOnHydrationMismatchIfConcurrentMode(fiber); // Nothing to hydrate. Make it an insertion. insertNonHydratedInstance((hydrationParentFiber: any), fiber); isHydrating = false; @@ -327,6 +335,7 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void { } const firstAttemptedInstance = nextInstance; if (!tryHydrate(fiber, nextInstance)) { + throwOnHydrationMismatchIfConcurrentMode(fiber); // If we can't hydrate this instance let's try the next one. // We use this as a heuristic. It's based on intuition and not data so it // might be flawed or unnecessary. diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 85728d96bb730..93cdd46ac5b4f 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -268,158 +268,164 @@ function resetSuspendedComponent(sourceFiber: Fiber, rootRenderLanes: Lanes) { } } -function markNearestSuspenseBoundaryShouldCapture( - returnFiber: Fiber, - sourceFiber: Fiber, - root: FiberRoot, - rootRenderLanes: Lanes, -): Fiber | null { +function getNearestSuspenseBoundaryToCapture(returnFiber: Fiber) { + let node = returnFiber; const hasInvisibleParentBoundary = hasSuspenseContext( suspenseStackCursor.current, (InvisibleParentSuspenseContext: SuspenseContext), ); - let node = returnFiber; do { if ( node.tag === SuspenseComponent && shouldCaptureSuspense(node, hasInvisibleParentBoundary) ) { - // Found the nearest boundary. - const suspenseBoundary = node; + return node; + } + // This boundary already captured during this render. Continue to the next + // boundary. + node = node.return; + } while (node !== null); + return null; +} - // This marks a Suspense boundary so that when we're unwinding the stack, - // it captures the suspended "exception" and does a second (fallback) pass. +function markNearestSuspenseBoundaryShouldCapture( + returnFiber: Fiber, + sourceFiber: Fiber, + root: FiberRoot, + rootRenderLanes: Lanes, +): Fiber | null { + const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); - if ((suspenseBoundary.mode & ConcurrentMode) === NoMode) { - // Legacy Mode Suspense - // - // If the boundary is in legacy mode, we should *not* - // suspend the commit. Pretend as if the suspended component rendered - // null and keep rendering. When the Suspense boundary completes, - // we'll do a second pass to render the fallback. - if (suspenseBoundary === returnFiber) { - // Special case where we suspended while reconciling the children of - // a Suspense boundary's inner Offscreen wrapper fiber. This happens - // when a React.lazy component is a direct child of a - // Suspense boundary. - // - // Suspense boundaries are implemented as multiple fibers, but they - // are a single conceptual unit. The legacy mode behavior where we - // pretend the suspended fiber committed as `null` won't work, - // because in this case the "suspended" fiber is the inner - // Offscreen wrapper. - // - // Because the contents of the boundary haven't started rendering - // yet (i.e. nothing in the tree has partially rendered) we can - // switch to the regular, concurrent mode behavior: mark the - // boundary with ShouldCapture and enter the unwind phase. - suspenseBoundary.flags |= ShouldCapture; - } else { - suspenseBoundary.flags |= DidCapture; - sourceFiber.flags |= ForceUpdateForLegacySuspense; + if (!suspenseBoundary) { + // Could not find a Suspense boundary capable of capturing. + return null; + } - // We're going to commit this fiber even though it didn't complete. - // But we shouldn't call any lifecycle methods or callbacks. Remove - // all lifecycle effect tags. - sourceFiber.flags &= ~(LifecycleEffectMask | Incomplete); + // This marks a Suspense boundary so that when we're unwinding the stack, + // it captures the suspended "exception" and does a second (fallback) pass. - if (supportsPersistence && enablePersistentOffscreenHostContainer) { - // Another legacy Suspense quirk. In persistent mode, if this is the - // initial mount, override the props of the host container to hide - // its contents. - const currentSuspenseBoundary = suspenseBoundary.alternate; - if (currentSuspenseBoundary === null) { - const offscreenFiber: Fiber = (suspenseBoundary.child: any); - const offscreenContainer = offscreenFiber.child; - if (offscreenContainer !== null) { - const children = offscreenContainer.memoizedProps.children; - const containerProps = getOffscreenContainerProps( - 'hidden', - children, - ); - offscreenContainer.pendingProps = containerProps; - offscreenContainer.memoizedProps = containerProps; - } - } - } + if ((suspenseBoundary.mode & ConcurrentMode) === NoMode) { + // Legacy Mode Suspense + // + // If the boundary is in legacy mode, we should *not* + // suspend the commit. Pretend as if the suspended component rendered + // null and keep rendering. When the Suspense boundary completes, + // we'll do a second pass to render the fallback. + if (suspenseBoundary === returnFiber) { + // Special case where we suspended while reconciling the children of + // a Suspense boundary's inner Offscreen wrapper fiber. This happens + // when a React.lazy component is a direct child of a + // Suspense boundary. + // + // Suspense boundaries are implemented as multiple fibers, but they + // are a single conceptual unit. The legacy mode behavior where we + // pretend the suspended fiber committed as `null` won't work, + // because in this case the "suspended" fiber is the inner + // Offscreen wrapper. + // + // Because the contents of the boundary haven't started rendering + // yet (i.e. nothing in the tree has partially rendered) we can + // switch to the regular, concurrent mode behavior: mark the + // boundary with ShouldCapture and enter the unwind phase. + suspenseBoundary.flags |= ShouldCapture; + } else { + suspenseBoundary.flags |= DidCapture; + sourceFiber.flags |= ForceUpdateForLegacySuspense; + + // We're going to commit this fiber even though it didn't complete. + // But we shouldn't call any lifecycle methods or callbacks. Remove + // all lifecycle effect tags. + sourceFiber.flags &= ~(LifecycleEffectMask | Incomplete); - if (sourceFiber.tag === ClassComponent) { - const currentSourceFiber = sourceFiber.alternate; - if (currentSourceFiber === null) { - // This is a new mount. Change the tag so it's not mistaken for a - // completed class component. For example, we should not call - // componentWillUnmount if it is deleted. - sourceFiber.tag = IncompleteClassComponent; - } else { - // When we try rendering again, we should not reuse the current fiber, - // since it's known to be in an inconsistent state. Use a force update to - // prevent a bail out. - const update = createUpdate(NoTimestamp, SyncLane); - update.tag = ForceUpdate; - enqueueUpdate(sourceFiber, update, SyncLane); - } + if (supportsPersistence && enablePersistentOffscreenHostContainer) { + // Another legacy Suspense quirk. In persistent mode, if this is the + // initial mount, override the props of the host container to hide + // its contents. + const currentSuspenseBoundary = suspenseBoundary.alternate; + if (currentSuspenseBoundary === null) { + const offscreenFiber: Fiber = (suspenseBoundary.child: any); + const offscreenContainer = offscreenFiber.child; + if (offscreenContainer !== null) { + const children = offscreenContainer.memoizedProps.children; + const containerProps = getOffscreenContainerProps( + 'hidden', + children, + ); + offscreenContainer.pendingProps = containerProps; + offscreenContainer.memoizedProps = containerProps; } + } + } - // The source fiber did not complete. Mark it with Sync priority to - // indicate that it still has pending work. - sourceFiber.lanes = mergeLanes(sourceFiber.lanes, SyncLane); + if (sourceFiber.tag === ClassComponent) { + const currentSourceFiber = sourceFiber.alternate; + if (currentSourceFiber === null) { + // This is a new mount. Change the tag so it's not mistaken for a + // completed class component. For example, we should not call + // componentWillUnmount if it is deleted. + sourceFiber.tag = IncompleteClassComponent; + } else { + // When we try rendering again, we should not reuse the current fiber, + // since it's known to be in an inconsistent state. Use a force update to + // prevent a bail out. + const update = createUpdate(NoTimestamp, SyncLane); + update.tag = ForceUpdate; + enqueueUpdate(sourceFiber, update, SyncLane); } - return suspenseBoundary; } - // Confirmed that the boundary is in a concurrent mode tree. Continue - // with the normal suspend path. - // - // After this we'll use a set of heuristics to determine whether this - // render pass will run to completion or restart or "suspend" the commit. - // The actual logic for this is spread out in different places. - // - // This first principle is that if we're going to suspend when we complete - // a root, then we should also restart if we get an update or ping that - // might unsuspend it, and vice versa. The only reason to suspend is - // because you think you might want to restart before committing. However, - // it doesn't make sense to restart only while in the period we're suspended. - // - // Restarting too aggressively is also not good because it starves out any - // intermediate loading state. So we use heuristics to determine when. - // Suspense Heuristics - // - // If nothing threw a Promise or all the same fallbacks are already showing, - // then don't suspend/restart. - // - // If this is an initial render of a new tree of Suspense boundaries and - // those trigger a fallback, then don't suspend/restart. We want to ensure - // that we can show the initial loading state as quickly as possible. - // - // If we hit a "Delayed" case, such as when we'd switch from content back into - // a fallback, then we should always suspend/restart. Transitions apply - // to this case. If none is defined, JND is used instead. - // - // If we're already showing a fallback and it gets "retried", allowing us to show - // another level, but there's still an inner boundary that would show a fallback, - // then we suspend/restart for 500ms since the last time we showed a fallback - // anywhere in the tree. This effectively throttles progressive loading into a - // consistent train of commits. This also gives us an opportunity to restart to - // get to the completed state slightly earlier. - // - // If there's ambiguity due to batching it's resolved in preference of: - // 1) "delayed", 2) "initial render", 3) "retry". - // - // We want to ensure that a "busy" state doesn't get force committed. We want to - // ensure that new initial loading states can commit as soon as possible. - suspenseBoundary.flags |= ShouldCapture; - // TODO: I think we can remove this, since we now use `DidCapture` in - // the begin phase to prevent an early bailout. - suspenseBoundary.lanes = rootRenderLanes; - return suspenseBoundary; + // The source fiber did not complete. Mark it with Sync priority to + // indicate that it still has pending work. + sourceFiber.lanes = mergeLanes(sourceFiber.lanes, SyncLane); } - // This boundary already captured during this render. Continue to the next - // boundary. - node = node.return; - } while (node !== null); + return suspenseBoundary; + } + // Confirmed that the boundary is in a concurrent mode tree. Continue + // with the normal suspend path. + // + // After this we'll use a set of heuristics to determine whether this + // render pass will run to completion or restart or "suspend" the commit. + // The actual logic for this is spread out in different places. + // + // This first principle is that if we're going to suspend when we complete + // a root, then we should also restart if we get an update or ping that + // might unsuspend it, and vice versa. The only reason to suspend is + // because you think you might want to restart before committing. However, + // it doesn't make sense to restart only while in the period we're suspended. + // + // Restarting too aggressively is also not good because it starves out any + // intermediate loading state. So we use heuristics to determine when. - // Could not find a Suspense boundary capable of capturing. - return null; + // Suspense Heuristics + // + // If nothing threw a Promise or all the same fallbacks are already showing, + // then don't suspend/restart. + // + // If this is an initial render of a new tree of Suspense boundaries and + // those trigger a fallback, then don't suspend/restart. We want to ensure + // that we can show the initial loading state as quickly as possible. + // + // If we hit a "Delayed" case, such as when we'd switch from content back into + // a fallback, then we should always suspend/restart. Transitions apply + // to this case. If none is defined, JND is used instead. + // + // If we're already showing a fallback and it gets "retried", allowing us to show + // another level, but there's still an inner boundary that would show a fallback, + // then we suspend/restart for 500ms since the last time we showed a fallback + // anywhere in the tree. This effectively throttles progressive loading into a + // consistent train of commits. This also gives us an opportunity to restart to + // get to the completed state slightly earlier. + // + // If there's ambiguity due to batching it's resolved in preference of: + // 1) "delayed", 2) "initial render", 3) "retry". + // + // We want to ensure that a "busy" state doesn't get force committed. We want to + // ensure that new initial loading states can commit as soon as possible. + suspenseBoundary.flags |= ShouldCapture; + // TODO: I think we can remove this, since we now use `DidCapture` in + // the begin phase to prevent an early bailout. + suspenseBoundary.lanes = rootRenderLanes; + return suspenseBoundary; } function throwException( @@ -487,22 +493,30 @@ function throwException( } else { // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); + const didCaptureBefore = + (suspenseBoundary !== null && + suspenseBoundary.flags & ShouldCapture) !== NoFlags; // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. // Instead of surfacing the error, find the nearest Suspense boundary // and render it again without hydration. - const suspenseBoundary = markNearestSuspenseBoundaryShouldCapture( + markNearestSuspenseBoundaryShouldCapture( returnFiber, sourceFiber, root, rootRenderLanes, ); - if (suspenseBoundary !== null) { + if (suspenseBoundary !== null && !didCaptureBefore) { // Set a flag to indicate that we should try rendering the normal // children again, not the fallback. suspenseBoundary.flags |= ForceClientRender; return; } + if (didCaptureBefore && value && value.message === 'Hydration mismatch') { + // swallow mismatch error + return; + } } else { // Otherwise, fall through to the error path. } diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 27b0719ba8532..65981c2d7a448 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -268,158 +268,164 @@ function resetSuspendedComponent(sourceFiber: Fiber, rootRenderLanes: Lanes) { } } -function markNearestSuspenseBoundaryShouldCapture( - returnFiber: Fiber, - sourceFiber: Fiber, - root: FiberRoot, - rootRenderLanes: Lanes, -): Fiber | null { +function getNearestSuspenseBoundaryToCapture(returnFiber: Fiber) { + let node = returnFiber; const hasInvisibleParentBoundary = hasSuspenseContext( suspenseStackCursor.current, (InvisibleParentSuspenseContext: SuspenseContext), ); - let node = returnFiber; do { if ( node.tag === SuspenseComponent && shouldCaptureSuspense(node, hasInvisibleParentBoundary) ) { - // Found the nearest boundary. - const suspenseBoundary = node; + return node; + } + // This boundary already captured during this render. Continue to the next + // boundary. + node = node.return; + } while (node !== null); + return null; +} - // This marks a Suspense boundary so that when we're unwinding the stack, - // it captures the suspended "exception" and does a second (fallback) pass. +function markNearestSuspenseBoundaryShouldCapture( + returnFiber: Fiber, + sourceFiber: Fiber, + root: FiberRoot, + rootRenderLanes: Lanes, +): Fiber | null { + const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); - if ((suspenseBoundary.mode & ConcurrentMode) === NoMode) { - // Legacy Mode Suspense - // - // If the boundary is in legacy mode, we should *not* - // suspend the commit. Pretend as if the suspended component rendered - // null and keep rendering. When the Suspense boundary completes, - // we'll do a second pass to render the fallback. - if (suspenseBoundary === returnFiber) { - // Special case where we suspended while reconciling the children of - // a Suspense boundary's inner Offscreen wrapper fiber. This happens - // when a React.lazy component is a direct child of a - // Suspense boundary. - // - // Suspense boundaries are implemented as multiple fibers, but they - // are a single conceptual unit. The legacy mode behavior where we - // pretend the suspended fiber committed as `null` won't work, - // because in this case the "suspended" fiber is the inner - // Offscreen wrapper. - // - // Because the contents of the boundary haven't started rendering - // yet (i.e. nothing in the tree has partially rendered) we can - // switch to the regular, concurrent mode behavior: mark the - // boundary with ShouldCapture and enter the unwind phase. - suspenseBoundary.flags |= ShouldCapture; - } else { - suspenseBoundary.flags |= DidCapture; - sourceFiber.flags |= ForceUpdateForLegacySuspense; + if (!suspenseBoundary) { + // Could not find a Suspense boundary capable of capturing. + return null; + } - // We're going to commit this fiber even though it didn't complete. - // But we shouldn't call any lifecycle methods or callbacks. Remove - // all lifecycle effect tags. - sourceFiber.flags &= ~(LifecycleEffectMask | Incomplete); + // This marks a Suspense boundary so that when we're unwinding the stack, + // it captures the suspended "exception" and does a second (fallback) pass. - if (supportsPersistence && enablePersistentOffscreenHostContainer) { - // Another legacy Suspense quirk. In persistent mode, if this is the - // initial mount, override the props of the host container to hide - // its contents. - const currentSuspenseBoundary = suspenseBoundary.alternate; - if (currentSuspenseBoundary === null) { - const offscreenFiber: Fiber = (suspenseBoundary.child: any); - const offscreenContainer = offscreenFiber.child; - if (offscreenContainer !== null) { - const children = offscreenContainer.memoizedProps.children; - const containerProps = getOffscreenContainerProps( - 'hidden', - children, - ); - offscreenContainer.pendingProps = containerProps; - offscreenContainer.memoizedProps = containerProps; - } - } - } + if ((suspenseBoundary.mode & ConcurrentMode) === NoMode) { + // Legacy Mode Suspense + // + // If the boundary is in legacy mode, we should *not* + // suspend the commit. Pretend as if the suspended component rendered + // null and keep rendering. When the Suspense boundary completes, + // we'll do a second pass to render the fallback. + if (suspenseBoundary === returnFiber) { + // Special case where we suspended while reconciling the children of + // a Suspense boundary's inner Offscreen wrapper fiber. This happens + // when a React.lazy component is a direct child of a + // Suspense boundary. + // + // Suspense boundaries are implemented as multiple fibers, but they + // are a single conceptual unit. The legacy mode behavior where we + // pretend the suspended fiber committed as `null` won't work, + // because in this case the "suspended" fiber is the inner + // Offscreen wrapper. + // + // Because the contents of the boundary haven't started rendering + // yet (i.e. nothing in the tree has partially rendered) we can + // switch to the regular, concurrent mode behavior: mark the + // boundary with ShouldCapture and enter the unwind phase. + suspenseBoundary.flags |= ShouldCapture; + } else { + suspenseBoundary.flags |= DidCapture; + sourceFiber.flags |= ForceUpdateForLegacySuspense; + + // We're going to commit this fiber even though it didn't complete. + // But we shouldn't call any lifecycle methods or callbacks. Remove + // all lifecycle effect tags. + sourceFiber.flags &= ~(LifecycleEffectMask | Incomplete); - if (sourceFiber.tag === ClassComponent) { - const currentSourceFiber = sourceFiber.alternate; - if (currentSourceFiber === null) { - // This is a new mount. Change the tag so it's not mistaken for a - // completed class component. For example, we should not call - // componentWillUnmount if it is deleted. - sourceFiber.tag = IncompleteClassComponent; - } else { - // When we try rendering again, we should not reuse the current fiber, - // since it's known to be in an inconsistent state. Use a force update to - // prevent a bail out. - const update = createUpdate(NoTimestamp, SyncLane); - update.tag = ForceUpdate; - enqueueUpdate(sourceFiber, update, SyncLane); - } + if (supportsPersistence && enablePersistentOffscreenHostContainer) { + // Another legacy Suspense quirk. In persistent mode, if this is the + // initial mount, override the props of the host container to hide + // its contents. + const currentSuspenseBoundary = suspenseBoundary.alternate; + if (currentSuspenseBoundary === null) { + const offscreenFiber: Fiber = (suspenseBoundary.child: any); + const offscreenContainer = offscreenFiber.child; + if (offscreenContainer !== null) { + const children = offscreenContainer.memoizedProps.children; + const containerProps = getOffscreenContainerProps( + 'hidden', + children, + ); + offscreenContainer.pendingProps = containerProps; + offscreenContainer.memoizedProps = containerProps; } + } + } - // The source fiber did not complete. Mark it with Sync priority to - // indicate that it still has pending work. - sourceFiber.lanes = mergeLanes(sourceFiber.lanes, SyncLane); + if (sourceFiber.tag === ClassComponent) { + const currentSourceFiber = sourceFiber.alternate; + if (currentSourceFiber === null) { + // This is a new mount. Change the tag so it's not mistaken for a + // completed class component. For example, we should not call + // componentWillUnmount if it is deleted. + sourceFiber.tag = IncompleteClassComponent; + } else { + // When we try rendering again, we should not reuse the current fiber, + // since it's known to be in an inconsistent state. Use a force update to + // prevent a bail out. + const update = createUpdate(NoTimestamp, SyncLane); + update.tag = ForceUpdate; + enqueueUpdate(sourceFiber, update, SyncLane); } - return suspenseBoundary; } - // Confirmed that the boundary is in a concurrent mode tree. Continue - // with the normal suspend path. - // - // After this we'll use a set of heuristics to determine whether this - // render pass will run to completion or restart or "suspend" the commit. - // The actual logic for this is spread out in different places. - // - // This first principle is that if we're going to suspend when we complete - // a root, then we should also restart if we get an update or ping that - // might unsuspend it, and vice versa. The only reason to suspend is - // because you think you might want to restart before committing. However, - // it doesn't make sense to restart only while in the period we're suspended. - // - // Restarting too aggressively is also not good because it starves out any - // intermediate loading state. So we use heuristics to determine when. - // Suspense Heuristics - // - // If nothing threw a Promise or all the same fallbacks are already showing, - // then don't suspend/restart. - // - // If this is an initial render of a new tree of Suspense boundaries and - // those trigger a fallback, then don't suspend/restart. We want to ensure - // that we can show the initial loading state as quickly as possible. - // - // If we hit a "Delayed" case, such as when we'd switch from content back into - // a fallback, then we should always suspend/restart. Transitions apply - // to this case. If none is defined, JND is used instead. - // - // If we're already showing a fallback and it gets "retried", allowing us to show - // another level, but there's still an inner boundary that would show a fallback, - // then we suspend/restart for 500ms since the last time we showed a fallback - // anywhere in the tree. This effectively throttles progressive loading into a - // consistent train of commits. This also gives us an opportunity to restart to - // get to the completed state slightly earlier. - // - // If there's ambiguity due to batching it's resolved in preference of: - // 1) "delayed", 2) "initial render", 3) "retry". - // - // We want to ensure that a "busy" state doesn't get force committed. We want to - // ensure that new initial loading states can commit as soon as possible. - suspenseBoundary.flags |= ShouldCapture; - // TODO: I think we can remove this, since we now use `DidCapture` in - // the begin phase to prevent an early bailout. - suspenseBoundary.lanes = rootRenderLanes; - return suspenseBoundary; + // The source fiber did not complete. Mark it with Sync priority to + // indicate that it still has pending work. + sourceFiber.lanes = mergeLanes(sourceFiber.lanes, SyncLane); } - // This boundary already captured during this render. Continue to the next - // boundary. - node = node.return; - } while (node !== null); + return suspenseBoundary; + } + // Confirmed that the boundary is in a concurrent mode tree. Continue + // with the normal suspend path. + // + // After this we'll use a set of heuristics to determine whether this + // render pass will run to completion or restart or "suspend" the commit. + // The actual logic for this is spread out in different places. + // + // This first principle is that if we're going to suspend when we complete + // a root, then we should also restart if we get an update or ping that + // might unsuspend it, and vice versa. The only reason to suspend is + // because you think you might want to restart before committing. However, + // it doesn't make sense to restart only while in the period we're suspended. + // + // Restarting too aggressively is also not good because it starves out any + // intermediate loading state. So we use heuristics to determine when. - // Could not find a Suspense boundary capable of capturing. - return null; + // Suspense Heuristics + // + // If nothing threw a Promise or all the same fallbacks are already showing, + // then don't suspend/restart. + // + // If this is an initial render of a new tree of Suspense boundaries and + // those trigger a fallback, then don't suspend/restart. We want to ensure + // that we can show the initial loading state as quickly as possible. + // + // If we hit a "Delayed" case, such as when we'd switch from content back into + // a fallback, then we should always suspend/restart. Transitions apply + // to this case. If none is defined, JND is used instead. + // + // If we're already showing a fallback and it gets "retried", allowing us to show + // another level, but there's still an inner boundary that would show a fallback, + // then we suspend/restart for 500ms since the last time we showed a fallback + // anywhere in the tree. This effectively throttles progressive loading into a + // consistent train of commits. This also gives us an opportunity to restart to + // get to the completed state slightly earlier. + // + // If there's ambiguity due to batching it's resolved in preference of: + // 1) "delayed", 2) "initial render", 3) "retry". + // + // We want to ensure that a "busy" state doesn't get force committed. We want to + // ensure that new initial loading states can commit as soon as possible. + suspenseBoundary.flags |= ShouldCapture; + // TODO: I think we can remove this, since we now use `DidCapture` in + // the begin phase to prevent an early bailout. + suspenseBoundary.lanes = rootRenderLanes; + return suspenseBoundary; } function throwException( @@ -487,22 +493,30 @@ function throwException( } else { // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); + const didCaptureBefore = + (suspenseBoundary !== null && + suspenseBoundary.flags & ShouldCapture) !== NoFlags; // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. // Instead of surfacing the error, find the nearest Suspense boundary // and render it again without hydration. - const suspenseBoundary = markNearestSuspenseBoundaryShouldCapture( + markNearestSuspenseBoundaryShouldCapture( returnFiber, sourceFiber, root, rootRenderLanes, ); - if (suspenseBoundary !== null) { + if (suspenseBoundary !== null && !didCaptureBefore) { // Set a flag to indicate that we should try rendering the normal // children again, not the fallback. suspenseBoundary.flags |= ForceClientRender; return; } + if (didCaptureBefore && value && value.message === 'Hydration mismatch') { + // swallow mismatch error + return; + } } else { // Otherwise, fall through to the error path. } From 4f8850c3b55089fc4db0b750da4c6f066e8f3ea0 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 10:29:44 -0400 Subject: [PATCH 02/14] remove debugger --- .../__tests__/ReactDOMServerPartialHydration-test.internal.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index fd0d329a1c590..a0f9e2bad4620 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -278,7 +278,6 @@ describe('ReactDOMServerPartialHydration', () => { resolve(); await promise; - debugger; expect(Scheduler).toFlushAndYield([ // first pass, mismatches at end 'Hello', From 8828ebcf36642a7813fc498b38c1860b36292035 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 13:15:13 -0400 Subject: [PATCH 03/14] update error message --- .../src/__tests__/ReactDOMFizzServer-test.js | 23 +++++++------------ .../ReactDOMServerIntegrationHooks-test.js | 12 ++-------- .../src/ReactFiberHydrationContext.new.js | 4 +++- .../src/ReactFiberHydrationContext.old.js | 4 +++- .../src/ReactFiberThrow.new.js | 10 ++++---- .../src/ReactFiberThrow.old.js | 10 ++++---- scripts/error-codes/codes.json | 3 ++- 7 files changed, 28 insertions(+), 38 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index a2ede1746d08a..512b893673164 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1666,8 +1666,6 @@ describe('ReactDOMFizzServer', () => { // @gate supportsNativeUseSyncExternalStore // @gate experimental it('calls getServerSnapshot instead of getSnapshot', async () => { - const ref = React.createRef(); - function getServerSnapshot() { return 'server'; } @@ -1692,7 +1690,7 @@ describe('ReactDOMFizzServer', () => { getServerSnapshot, ); return ( -
+
); @@ -1715,21 +1713,16 @@ describe('ReactDOMFizzServer', () => { }); expect(Scheduler).toHaveYielded(['server']); - const serverRenderedDiv = container.getElementsByTagName('div')[0]; - ReactDOM.hydrateRoot(container, ); - // The first paint uses the server snapshot - expect(Scheduler).toFlushUntilNextPaint(['server']); - expect(getVisibleChildren(container)).toEqual(
server
); - // Hydration succeeded - expect(ref.current).toEqual(serverRenderedDiv); - - // Asynchronously we detect that the store has changed on the client, - // and patch up the inconsistency - expect(Scheduler).toFlushUntilNextPaint(['client']); + expect(() => { + // The first paint switches to client rendering due to mismatch + expect(Scheduler).toFlushUntilNextPaint(['client']); + }).toErrorDev( + 'Warning: An error occurred during hydration. The server HTML was replaced with client content', + {withoutStack: true}, + ); expect(getVisibleChildren(container)).toEqual(
client
); - expect(ref.current).toEqual(serverRenderedDiv); }); // The selector implementation uses the lazy ref initialization pattern diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index ba2b22437bc1b..bea6e1cee0f6e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -1644,11 +1644,10 @@ describe('ReactDOMServerHooks', () => { // This is the wrong HTML string container.innerHTML = ''; - ReactDOM.createRoot(container, {hydrate: true}).render(); + ReactDOM.hydrateRoot(container, ); expect(() => Scheduler.unstable_flushAll()).toErrorDev( [ 'Warning: An error occurred during hydration. The server HTML was replaced with client content in
.', - 'Warning: Expected server HTML to contain a matching
in
.', ], {withoutStack: 1}, ); @@ -1732,14 +1731,7 @@ describe('ReactDOMServerHooks', () => { // This is the wrong HTML string container.innerHTML = ''; - ReactDOM.createRoot(container, {hydrate: true}).render(); - expect(() => Scheduler.unstable_flushAll()).toErrorDev( - [ - 'Warning: An error occurred during hydration. The server HTML was replaced with client content in
.', - 'Warning: Expected server HTML to contain a matching
in
.', - ], - {withoutStack: 1}, - ); + ReactDOM.hydrateRoot(container, ); }); it('useOpaqueIdentifier warns when there is a hydration error and we are using ID as a string', async () => { diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 6a342870ebb33..aa10531a86bc6 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -316,7 +316,9 @@ function tryHydrate(fiber, nextInstance) { function throwOnHydrationMismatchIfConcurrentMode(fiber) { if ((fiber.mode & ConcurrentMode) !== NoMode) { - throw new Error('Hydration mismatch'); + throw new Error( + 'An error occurred during hydration. The server HTML was replaced with client content', + ); } } diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index 95a8ea0f29d1e..6d12d0171ed4c 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -316,7 +316,9 @@ function tryHydrate(fiber, nextInstance) { function throwOnHydrationMismatchIfConcurrentMode(fiber) { if ((fiber.mode & ConcurrentMode) !== NoMode) { - throw new Error('Hydration mismatch'); + throw new Error( + 'An error occurred during hydration. The server HTML was replaced with client content', + ); } } diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 93cdd46ac5b4f..cce2122a46c50 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -494,9 +494,9 @@ function throwException( // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); - const didCaptureBefore = - (suspenseBoundary !== null && - suspenseBoundary.flags & ShouldCapture) !== NoFlags; + const hasShouldCapture = + suspenseBoundary !== null && + (suspenseBoundary.flags & ShouldCapture) !== NoFlags; // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. // Instead of surfacing the error, find the nearest Suspense boundary @@ -507,13 +507,13 @@ function throwException( root, rootRenderLanes, ); - if (suspenseBoundary !== null && !didCaptureBefore) { + if (suspenseBoundary !== null && !hasShouldCapture) { // Set a flag to indicate that we should try rendering the normal // children again, not the fallback. suspenseBoundary.flags |= ForceClientRender; return; } - if (didCaptureBefore && value && value.message === 'Hydration mismatch') { + if (hasShouldCapture && value && value.message === 'Hydration mismatch') { // swallow mismatch error return; } diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 65981c2d7a448..9a69be0247c14 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -494,9 +494,9 @@ function throwException( // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); - const didCaptureBefore = - (suspenseBoundary !== null && - suspenseBoundary.flags & ShouldCapture) !== NoFlags; + const hasShouldCapture = + suspenseBoundary !== null && + (suspenseBoundary.flags & ShouldCapture) !== NoFlags; // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. // Instead of surfacing the error, find the nearest Suspense boundary @@ -507,13 +507,13 @@ function throwException( root, rootRenderLanes, ); - if (suspenseBoundary !== null && !didCaptureBefore) { + if (suspenseBoundary !== null && !hasShouldCapture) { // Set a flag to indicate that we should try rendering the normal // children again, not the fallback. suspenseBoundary.flags |= ForceClientRender; return; } - if (didCaptureBefore && value && value.message === 'Hydration mismatch') { + if (hasShouldCapture && value && value.message === 'Hydration mismatch') { // swallow mismatch error return; } diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index aab5d62476cc3..957c11f54a6cb 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -402,5 +402,6 @@ "414": "Did not expect this call in production. This is a bug in React. Please file an issue.", "415": "Error parsing the data. It's probably an error code or network corruption.", "416": "This environment don't support binary chunks.", - "417": "React currently only supports piping to one writable stream." + "417": "React currently only supports piping to one writable stream.", + "418": "An error occurred during hydration. The server HTML was replaced with client content" } From a55dcafde45d4b001483726ae9db39dac46ee0d4 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 13:20:48 -0400 Subject: [PATCH 04/14] update error message part2... --- packages/react-reconciler/src/ReactFiberThrow.new.js | 7 ++++++- packages/react-reconciler/src/ReactFiberThrow.old.js | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index cce2122a46c50..53ba5321447a1 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -513,7 +513,12 @@ function throwException( suspenseBoundary.flags |= ForceClientRender; return; } - if (hasShouldCapture && value && value.message === 'Hydration mismatch') { + if ( + hasShouldCapture && + value && + value.message === + 'An error occurred during hydration. The server HTML was replaced with client content' + ) { // swallow mismatch error return; } diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 9a69be0247c14..7ff63254cc631 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -513,7 +513,12 @@ function throwException( suspenseBoundary.flags |= ForceClientRender; return; } - if (hasShouldCapture && value && value.message === 'Hydration mismatch') { + if ( + hasShouldCapture && + value && + value.message === + 'An error occurred during hydration. The server HTML was replaced with client content' + ) { // swallow mismatch error return; } From 0b6a14daf891c1945d798437621c22439c24c623 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 13:50:42 -0400 Subject: [PATCH 05/14] fix test? --- .../src/ReactFiberThrow.new.js | 19 ++++++------------- .../src/ReactFiberThrow.old.js | 19 ++++++------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 53ba5321447a1..3f3f2437f7288 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -507,19 +507,12 @@ function throwException( root, rootRenderLanes, ); - if (suspenseBoundary !== null && !hasShouldCapture) { - // Set a flag to indicate that we should try rendering the normal - // children again, not the fallback. - suspenseBoundary.flags |= ForceClientRender; - return; - } - if ( - hasShouldCapture && - value && - value.message === - 'An error occurred during hydration. The server HTML was replaced with client content' - ) { - // swallow mismatch error + if (suspenseBoundary !== null) { + if (!hasShouldCapture) { + // Set a flag to indicate that we should try rendering the normal + // children again, not the fallback. + suspenseBoundary.flags |= ForceClientRender; + } return; } } else { diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 7ff63254cc631..3aea338f7898a 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -507,19 +507,12 @@ function throwException( root, rootRenderLanes, ); - if (suspenseBoundary !== null && !hasShouldCapture) { - // Set a flag to indicate that we should try rendering the normal - // children again, not the fallback. - suspenseBoundary.flags |= ForceClientRender; - return; - } - if ( - hasShouldCapture && - value && - value.message === - 'An error occurred during hydration. The server HTML was replaced with client content' - ) { - // swallow mismatch error + if (suspenseBoundary !== null) { + if (!hasShouldCapture) { + // Set a flag to indicate that we should try rendering the normal + // children again, not the fallback. + suspenseBoundary.flags |= ForceClientRender; + } return; } } else { From 4ecbb47dccf1e2cbda788cae4a30588f77917b2a Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 14:06:30 -0400 Subject: [PATCH 06/14] test? :( --- .../src/__tests__/ReactDOMFizzServer-test.js | 14 +------------- .../ReactDOMServerIntegrationHooks-test.js | 6 ++++++ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 512b893673164..b8cb93cb19721 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1732,8 +1732,6 @@ describe('ReactDOMFizzServer', () => { it('calls getServerSnapshot instead of getSnapshot (with selector and isEqual)', async () => { // Same as previous test, but with a selector that returns a complex object // that is memoized with a custom `isEqual` function. - const ref = React.createRef(); - function getServerSnapshot() { return {env: 'server', other: 'unrelated'}; } @@ -1791,21 +1789,11 @@ describe('ReactDOMFizzServer', () => { }); expect(Scheduler).toHaveYielded(['server']); - const serverRenderedDiv = container.getElementsByTagName('div')[0]; - ReactDOM.hydrateRoot(container, ); - // The first paint uses the server snapshot - expect(Scheduler).toFlushUntilNextPaint(['server']); - expect(getVisibleChildren(container)).toEqual(
server
); - // Hydration succeeded - expect(ref.current).toEqual(serverRenderedDiv); - - // Asynchronously we detect that the store has changed on the client, - // and patch up the inconsistency + // The first paint uses the client due to mismatch forcing client render expect(Scheduler).toFlushUntilNextPaint(['client']); expect(getVisibleChildren(container)).toEqual(
client
); - expect(ref.current).toEqual(serverRenderedDiv); }); // @gate supportsNativeUseSyncExternalStore diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index bea6e1cee0f6e..a1403c3dc5d21 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -1732,6 +1732,12 @@ describe('ReactDOMServerHooks', () => { // This is the wrong HTML string container.innerHTML = ''; ReactDOM.hydrateRoot(container, ); + expect(() => Scheduler.unstable_flushAll()).toErrorDev( + [ + 'Warning: An error occurred during hydration. The server HTML was replaced with client content in
.', + ], + {withoutStack: 1}, + ); }); it('useOpaqueIdentifier warns when there is a hydration error and we are using ID as a string', async () => { From 2cf8d3861f43fedca0caa41ee0e205eb0acd1a61 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 14:23:10 -0400 Subject: [PATCH 07/14] tests 4real --- .../src/__tests__/ReactDOMFizzServer-test.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index b8cb93cb19721..708f0f32d067b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1732,6 +1732,8 @@ describe('ReactDOMFizzServer', () => { it('calls getServerSnapshot instead of getSnapshot (with selector and isEqual)', async () => { // Same as previous test, but with a selector that returns a complex object // that is memoized with a custom `isEqual` function. + const ref = React.createRef(); + function getServerSnapshot() { return {env: 'server', other: 'unrelated'}; } @@ -1787,12 +1789,19 @@ describe('ReactDOMFizzServer', () => { ); pipe(writable); }); + console.log(container.innerHTML); expect(Scheduler).toHaveYielded(['server']); ReactDOM.hydrateRoot(container, ); // The first paint uses the client due to mismatch forcing client render - expect(Scheduler).toFlushUntilNextPaint(['client']); + expect(() => { + // The first paint switches to client rendering due to mismatch + expect(Scheduler).toFlushUntilNextPaint(['client']); + }).toErrorDev( + 'Warning: An error occurred during hydration. The server HTML was replaced with client content', + {withoutStack: true}, + ); expect(getVisibleChildren(container)).toEqual(
client
); }); From b25e7d2391eb18770dbf846bdabeddf8fdcc1c21 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 14:49:02 -0400 Subject: [PATCH 08/14] remove useRefAccessWarning gating --- packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 708f0f32d067b..eb80528a5a156 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1726,7 +1726,6 @@ describe('ReactDOMFizzServer', () => { }); // The selector implementation uses the lazy ref initialization pattern - // @gate !(enableUseRefAccessWarning && __DEV__) // @gate supportsNativeUseSyncExternalStore // @gate experimental it('calls getServerSnapshot instead of getSnapshot (with selector and isEqual)', async () => { @@ -1789,7 +1788,6 @@ describe('ReactDOMFizzServer', () => { ); pipe(writable); }); - console.log(container.innerHTML); expect(Scheduler).toHaveYielded(['server']); ReactDOM.hydrateRoot(container, ); From 8d03a4672714111ba85ffff4feb1413ff0aee4bb Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 15:45:46 -0400 Subject: [PATCH 09/14] split markSuspenseBoundary and getNearestBoundary --- .../src/ReactFiberThrow.new.js | 43 ++++++++----------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 3f3f2437f7288..374ff6c9061cb 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -288,22 +288,15 @@ function getNearestSuspenseBoundaryToCapture(returnFiber: Fiber) { return null; } -function markNearestSuspenseBoundaryShouldCapture( +function markSuspenseBoundaryShouldCapture( + suspenseBoundary: Fiber, returnFiber: Fiber, sourceFiber: Fiber, root: FiberRoot, rootRenderLanes: Lanes, ): Fiber | null { - const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); - - if (!suspenseBoundary) { - // Could not find a Suspense boundary capable of capturing. - return null; - } - // This marks a Suspense boundary so that when we're unwinding the stack, // it captures the suspended "exception" and does a second (fallback) pass. - if ((suspenseBoundary.mode & ConcurrentMode) === NoMode) { // Legacy Mode Suspense // @@ -464,13 +457,15 @@ function throwException( } // Schedule the nearest Suspense to re-render the timed out view. - const suspenseBoundary = markNearestSuspenseBoundaryShouldCapture( - returnFiber, - sourceFiber, - root, - rootRenderLanes, - ); + const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); if (suspenseBoundary !== null) { + markSuspenseBoundaryShouldCapture( + suspenseBoundary, + returnFiber, + sourceFiber, + root, + rootRenderLanes, + ); attachWakeableListeners( suspenseBoundary, root, @@ -494,25 +489,23 @@ function throwException( // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); - const hasShouldCapture = - suspenseBoundary !== null && - (suspenseBoundary.flags & ShouldCapture) !== NoFlags; // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. // Instead of surfacing the error, find the nearest Suspense boundary // and render it again without hydration. - markNearestSuspenseBoundaryShouldCapture( - returnFiber, - sourceFiber, - root, - rootRenderLanes, - ); if (suspenseBoundary !== null) { - if (!hasShouldCapture) { + if ((suspenseBoundary.flags & ShouldCapture) === NoFlags) { // Set a flag to indicate that we should try rendering the normal // children again, not the fallback. suspenseBoundary.flags |= ForceClientRender; } + markSuspenseBoundaryShouldCapture( + suspenseBoundary, + returnFiber, + sourceFiber, + root, + rootRenderLanes, + ); return; } } else { From 6c9abd8735f63a83c206b399f3d9ca6f58e9347f Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 15:49:11 -0400 Subject: [PATCH 10/14] also assert html is correct --- .../ReactDOMServerPartialHydration-test.internal.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index a0f9e2bad4620..9aab3bf94536f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -261,6 +261,10 @@ describe('ReactDOMServerPartialHydration', () => { 'Component', ]); + expect(container.innerHTML).toBe( + 'Hello
Component
Component
Component
Component
', + ); + suspend = true; client = true; @@ -274,6 +278,11 @@ describe('ReactDOMServerPartialHydration', () => { ]); jest.runAllTimers(); + // Unchanged + expect(container.innerHTML).toBe( + 'Hello
Component
Component
Component
Component
', + ); + suspend = false; resolve(); await promise; @@ -292,6 +301,8 @@ describe('ReactDOMServerPartialHydration', () => { 'Component', 'Component', ]); + + // Client rendered - suspense comment nodes removed expect(container.innerHTML).toBe( 'Hello
Component
Component
Component
Mismatch
', ); From 3da4d94007bac2c7828a74de7cc7e3f2deb1b118 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 26 Oct 2021 16:00:28 -0400 Subject: [PATCH 11/14] replace-fork --- .../src/ReactFiberThrow.old.js | 43 ++++++++----------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 3aea338f7898a..08a3a9731a3b8 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -288,22 +288,15 @@ function getNearestSuspenseBoundaryToCapture(returnFiber: Fiber) { return null; } -function markNearestSuspenseBoundaryShouldCapture( +function markSuspenseBoundaryShouldCapture( + suspenseBoundary: Fiber, returnFiber: Fiber, sourceFiber: Fiber, root: FiberRoot, rootRenderLanes: Lanes, ): Fiber | null { - const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); - - if (!suspenseBoundary) { - // Could not find a Suspense boundary capable of capturing. - return null; - } - // This marks a Suspense boundary so that when we're unwinding the stack, // it captures the suspended "exception" and does a second (fallback) pass. - if ((suspenseBoundary.mode & ConcurrentMode) === NoMode) { // Legacy Mode Suspense // @@ -464,13 +457,15 @@ function throwException( } // Schedule the nearest Suspense to re-render the timed out view. - const suspenseBoundary = markNearestSuspenseBoundaryShouldCapture( - returnFiber, - sourceFiber, - root, - rootRenderLanes, - ); + const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); if (suspenseBoundary !== null) { + markSuspenseBoundaryShouldCapture( + suspenseBoundary, + returnFiber, + sourceFiber, + root, + rootRenderLanes, + ); attachWakeableListeners( suspenseBoundary, root, @@ -494,25 +489,23 @@ function throwException( // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); - const hasShouldCapture = - suspenseBoundary !== null && - (suspenseBoundary.flags & ShouldCapture) !== NoFlags; // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. // Instead of surfacing the error, find the nearest Suspense boundary // and render it again without hydration. - markNearestSuspenseBoundaryShouldCapture( - returnFiber, - sourceFiber, - root, - rootRenderLanes, - ); if (suspenseBoundary !== null) { - if (!hasShouldCapture) { + if ((suspenseBoundary.flags & ShouldCapture) === NoFlags) { // Set a flag to indicate that we should try rendering the normal // children again, not the fallback. suspenseBoundary.flags |= ForceClientRender; } + markSuspenseBoundaryShouldCapture( + suspenseBoundary, + returnFiber, + sourceFiber, + root, + rootRenderLanes, + ); return; } } else { From ae4066552520e13d2d0dfba30c0b5c3cb41e36a9 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 8 Nov 2021 07:13:21 -0500 Subject: [PATCH 12/14] also remove client render flag on suspend --- packages/react-reconciler/src/ReactFiberThrow.new.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 374ff6c9061cb..cd9931687ba5a 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -459,6 +459,7 @@ function throwException( // Schedule the nearest Suspense to re-render the timed out view. const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); if (suspenseBoundary !== null) { + suspenseBoundary.flags &= ~ForceClientRender; markSuspenseBoundaryShouldCapture( suspenseBoundary, returnFiber, From 6b48b450136fc461af770d15ccbabe7363c16fb7 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 8 Nov 2021 07:15:25 -0500 Subject: [PATCH 13/14] replace-fork --- packages/react-reconciler/src/ReactFiberThrow.old.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 08a3a9731a3b8..8f6d18a48dea3 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -459,6 +459,7 @@ function throwException( // Schedule the nearest Suspense to re-render the timed out view. const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); if (suspenseBoundary !== null) { + suspenseBoundary.flags &= ~ForceClientRender; markSuspenseBoundaryShouldCapture( suspenseBoundary, returnFiber, From 646472f5f2ca1442ba64f86f7d42bae693546548 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 8 Nov 2021 09:46:27 -0500 Subject: [PATCH 14/14] fix mismerge???? --- .../src/__tests__/ReactDOMFizzServer-test.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 5acb38df2341a..8c282e1bb730e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1680,20 +1680,16 @@ describe('ReactDOMFizzServer', () => { function getServerSnapshot() { return 'server'; } - function getClientSnapshot() { return 'client'; } - function subscribe() { return () => {}; } - function Child({text}) { Scheduler.unstable_yieldValue(text); return text; } - function App() { const value = useSyncExternalStore( subscribe, @@ -1706,14 +1702,12 @@ describe('ReactDOMFizzServer', () => {
); } - const loggedErrors = []; await act(async () => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream( , - { onError(x) { loggedErrors.push(x); @@ -1737,38 +1731,30 @@ describe('ReactDOMFizzServer', () => { }); // The selector implementation uses the lazy ref initialization pattern - // @gate supportsNativeUseSyncExternalStore // @gate experimental it('calls getServerSnapshot instead of getSnapshot (with selector and isEqual)', async () => { // Same as previous test, but with a selector that returns a complex object // that is memoized with a custom `isEqual` function. const ref = React.createRef(); - function getServerSnapshot() { return {env: 'server', other: 'unrelated'}; } - function getClientSnapshot() { return {env: 'client', other: 'unrelated'}; } - function selector({env}) { return {env}; } - function isEqual(a, b) { return a.env === b.env; } - function subscribe() { return () => {}; } - function Child({text}) { Scheduler.unstable_yieldValue(text); return text; } - function App() { const {env} = useSyncExternalStoreWithSelector( subscribe, @@ -1783,14 +1769,12 @@ describe('ReactDOMFizzServer', () => {
); } - const loggedErrors = []; await act(async () => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream( , - { onError(x) { loggedErrors.push(x);