From 38b30381826a8fe43f4392a2229ad38e129461e0 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 31 Mar 2022 21:57:19 +0100 Subject: [PATCH 1/5] Delay showing fallback if hydrating suspends --- .../src/__tests__/ReactDOMFizzServer-test.js | 28 +++++++-------- .../__tests__/ReactDOMHydrationDiff-test.js | 34 +++++-------------- ...DOMServerPartialHydration-test.internal.js | 5 +-- .../src/ReactFiberBeginWork.new.js | 1 + .../src/ReactFiberBeginWork.old.js | 1 + .../src/ReactFiberLane.new.js | 11 ++++-- .../src/ReactFiberLane.old.js | 11 ++++-- .../src/ReactFiberWorkLoop.new.js | 4 +-- .../src/ReactFiberWorkLoop.old.js | 4 +-- 9 files changed, 47 insertions(+), 52 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 568a931c7d9e3..a06d7c5d17c53 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -869,16 +869,16 @@ describe('ReactDOMFizzServer', () => { }); // We still can't render it on the client. - expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to an ' + - 'error during server rendering. Switched to client rendering.', - ]); + expect(Scheduler).toFlushAndYield([]); expect(getVisibleChildren(container)).toEqual(
Loading...
); // We now resolve it on the client. resolveText('Hello'); - Scheduler.unstable_flushAll(); + expect(Scheduler).toFlushAndYield([ + 'The server could not finish this Suspense boundary, likely due to an ' + + 'error during server rendering. Switched to client rendering.', + ]); // The client rendered HTML is now in place. expect(getVisibleChildren(container)).toEqual( @@ -2293,17 +2293,12 @@ describe('ReactDOMFizzServer', () => { }, }); - // An error logged but instead of surfacing it to the UI, we switched - // to client rendering. - expect(Scheduler).toFlushAndYield([ - 'Hydration error', - 'There was an error while hydrating this Suspense boundary. Switched ' + - 'to client rendering.', - ]); + // An error happened but instead of surfacing it to the UI, we suspended. + expect(Scheduler).toFlushAndYield([]); expect(getVisibleChildren(container)).toEqual(
- Loading... + Yay!
, ); @@ -2311,7 +2306,12 @@ describe('ReactDOMFizzServer', () => { await act(async () => { resolveText('Yay!'); }); - expect(Scheduler).toFlushAndYield(['Yay!']); + expect(Scheduler).toFlushAndYield([ + 'Yay!', + 'Hydration error', + 'There was an error while hydrating this Suspense boundary. Switched ' + + 'to client rendering.', + ]); expect(getVisibleChildren(container)).toEqual(
diff --git a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js index f79ca0c18b631..a6a1a99890388 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js @@ -1046,7 +1046,7 @@ describe('ReactDOMServerHydration', () => { }); // @gate __DEV__ - it('warns when client renders an extra node inside Suspense fallback', () => { + it('does not warn when client renders an extra node inside Suspense fallback', () => { function Mismatch({isClient}) { return (
@@ -1063,27 +1063,18 @@ describe('ReactDOMServerHydration', () => {
); } - // TODO: Why does this not show a fallback mismatch? - // And why is this message different from the other ones? if ( gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch) ) { - expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` - Array [ - "Caught [The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.]", - ] - `); + // There is no error because we don't actually hydrate fallbacks. + expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`Array []`); } else { - expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` - Array [ - "Caught [The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.]", - ] - `); + expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`Array []`); } }); // @gate __DEV__ - it('warns when server renders an extra node inside Suspense fallback', () => { + it('does not warn when server renders an extra node inside Suspense fallback', () => { function Mismatch({isClient}) { return (
@@ -1100,22 +1091,13 @@ describe('ReactDOMServerHydration', () => {
); } - // TODO: Why does this not show a fallback mismatch? - // And why is this message different from the other ones? if ( gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch) ) { - expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` - Array [ - "Caught [The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.]", - ] - `); + // There is no error because we don't actually hydrate fallbacks. + expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`Array []`); } else { - expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` - Array [ - "Caught [The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.]", - ] - `); + expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`Array []`); } }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 697ce84fe7bd3..66bfdf9a39ed4 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -2137,10 +2137,7 @@ describe('ReactDOMServerPartialHydration', () => { }); suspend = true; - expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to ' + - 'an error during server rendering. Switched to client rendering.', - ]); + expect(Scheduler).toFlushAndYield([]); // We haven't hydrated the second child but the placeholder is still in the list. expect(container.textContent).toBe('ALoading B'); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 6fee8d948ebe2..32da430eef071 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2241,6 +2241,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { } else { // Suspended but we should no longer be in dehydrated mode. // Therefore we now have to render the fallback. + renderDidSuspendDelayIfPossible(); const nextPrimaryChildren = nextProps.children; const nextFallbackChildren = nextProps.fallback; const fallbackChildFragment = mountSuspenseFallbackAfterRetryWithoutHydrating( diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index fc4912e7ec393..45aa1cd43ba91 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -2241,6 +2241,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { } else { // Suspended but we should no longer be in dehydrated mode. // Therefore we now have to render the fallback. + renderDidSuspendDelayIfPossible(); const nextPrimaryChildren = nextProps.children; const nextFallbackChildren = nextProps.fallback; const fallbackChildFragment = mountSuspenseFallbackAfterRetryWithoutHydrating( diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index 597f996331999..a61ab9188d2af 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -458,8 +458,15 @@ export function includesNonIdleWork(lanes: Lanes) { export function includesOnlyRetries(lanes: Lanes) { return (lanes & RetryLanes) === lanes; } -export function includesOnlyTransitions(lanes: Lanes) { - return (lanes & TransitionLanes) === lanes; +export function includesOnlyTransitionsOrHydration(lanes: Lanes) { + const TransitionOrHydrationLanes = + TransitionLanes | + InputContinuousHydrationLane | + DefaultHydrationLane | + TransitionHydrationLane | + SelectiveHydrationLane | + IdleHydrationLane; + return (lanes & TransitionOrHydrationLanes) !== NoLanes; } export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index 9f366c9ade886..961bc69ca961f 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -458,8 +458,15 @@ export function includesNonIdleWork(lanes: Lanes) { export function includesOnlyRetries(lanes: Lanes) { return (lanes & RetryLanes) === lanes; } -export function includesOnlyTransitions(lanes: Lanes) { - return (lanes & TransitionLanes) === lanes; +export function includesOnlyTransitionsOrHydration(lanes: Lanes) { + const TransitionOrHydrationLanes = + TransitionLanes | + InputContinuousHydrationLane | + DefaultHydrationLane | + TransitionHydrationLane | + SelectiveHydrationLane | + IdleHydrationLane; + return (lanes & TransitionOrHydrationLanes) !== NoLanes; } export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 558440effa77a..1c9aab3b39097 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -132,7 +132,7 @@ import { pickArbitraryLane, includesNonIdleWork, includesOnlyRetries, - includesOnlyTransitions, + includesOnlyTransitionsOrHydration, includesBlockingLane, includesExpiredLane, getNextLanes, @@ -1110,7 +1110,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootSuspendedWithDelay: { markRootSuspended(root, lanes); - if (includesOnlyTransitions(lanes)) { + if (includesOnlyTransitionsOrHydration(lanes)) { // This is a transition, so we should exit without committing a // placeholder and without scheduling a timeout. Delay indefinitely // until we receive more data. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index c1c090d82b0b5..8ca44dacf6e9a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -132,7 +132,7 @@ import { pickArbitraryLane, includesNonIdleWork, includesOnlyRetries, - includesOnlyTransitions, + includesOnlyTransitionsOrHydration, includesBlockingLane, includesExpiredLane, getNextLanes, @@ -1110,7 +1110,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootSuspendedWithDelay: { markRootSuspended(root, lanes); - if (includesOnlyTransitions(lanes)) { + if (includesOnlyTransitionsOrHydration(lanes)) { // This is a transition, so we should exit without committing a // placeholder and without scheduling a timeout. Delay indefinitely // until we receive more data. From a47c615dac939b35357d8e02d31cb9413c4bed98 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 31 Mar 2022 22:07:49 +0100 Subject: [PATCH 2/5] Fix up --- packages/react-reconciler/src/ReactFiberLane.new.js | 2 +- packages/react-reconciler/src/ReactFiberLane.old.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index a61ab9188d2af..dd9ace002935d 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -466,7 +466,7 @@ export function includesOnlyTransitionsOrHydration(lanes: Lanes) { TransitionHydrationLane | SelectiveHydrationLane | IdleHydrationLane; - return (lanes & TransitionOrHydrationLanes) !== NoLanes; + return (lanes & TransitionOrHydrationLanes) === lanes; } export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index 961bc69ca961f..8a8ff62b48344 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -466,7 +466,7 @@ export function includesOnlyTransitionsOrHydration(lanes: Lanes) { TransitionHydrationLane | SelectiveHydrationLane | IdleHydrationLane; - return (lanes & TransitionOrHydrationLanes) !== NoLanes; + return (lanes & TransitionOrHydrationLanes) === lanes; } export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { From 6f96d0ad69d16b949cae3ca1f0118cec1553cc9c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 1 Apr 2022 00:18:12 +0100 Subject: [PATCH 3/5] Include all non-urgent lanes --- packages/react-reconciler/src/ReactFiberLane.new.js | 12 +++--------- packages/react-reconciler/src/ReactFiberLane.old.js | 12 +++--------- .../react-reconciler/src/ReactFiberWorkLoop.new.js | 4 ++-- .../react-reconciler/src/ReactFiberWorkLoop.old.js | 4 ++-- 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index dd9ace002935d..3bf65a9093add 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -458,15 +458,9 @@ export function includesNonIdleWork(lanes: Lanes) { export function includesOnlyRetries(lanes: Lanes) { return (lanes & RetryLanes) === lanes; } -export function includesOnlyTransitionsOrHydration(lanes: Lanes) { - const TransitionOrHydrationLanes = - TransitionLanes | - InputContinuousHydrationLane | - DefaultHydrationLane | - TransitionHydrationLane | - SelectiveHydrationLane | - IdleHydrationLane; - return (lanes & TransitionOrHydrationLanes) === lanes; +export function includesOnlyNonUrgentLanes(lanes: Lanes) { + const UrgentLanes = SyncLane | InputContinuousLane | DefaultLane; + return (lanes & UrgentLanes) === NoLanes; } export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index 8a8ff62b48344..ec884de182ff6 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -458,15 +458,9 @@ export function includesNonIdleWork(lanes: Lanes) { export function includesOnlyRetries(lanes: Lanes) { return (lanes & RetryLanes) === lanes; } -export function includesOnlyTransitionsOrHydration(lanes: Lanes) { - const TransitionOrHydrationLanes = - TransitionLanes | - InputContinuousHydrationLane | - DefaultHydrationLane | - TransitionHydrationLane | - SelectiveHydrationLane | - IdleHydrationLane; - return (lanes & TransitionOrHydrationLanes) === lanes; +export function includesOnlyNonUrgentLanes(lanes: Lanes) { + const UrgentLanes = SyncLane | InputContinuousLane | DefaultLane; + return (lanes & UrgentLanes) === NoLanes; } export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 1c9aab3b39097..3e50d6e391608 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -132,7 +132,7 @@ import { pickArbitraryLane, includesNonIdleWork, includesOnlyRetries, - includesOnlyTransitionsOrHydration, + includesOnlyNonUrgentLanes, includesBlockingLane, includesExpiredLane, getNextLanes, @@ -1110,7 +1110,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootSuspendedWithDelay: { markRootSuspended(root, lanes); - if (includesOnlyTransitionsOrHydration(lanes)) { + if (includesOnlyNonUrgentLanes(lanes)) { // This is a transition, so we should exit without committing a // placeholder and without scheduling a timeout. Delay indefinitely // until we receive more data. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 8ca44dacf6e9a..2745bb87c09eb 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -132,7 +132,7 @@ import { pickArbitraryLane, includesNonIdleWork, includesOnlyRetries, - includesOnlyTransitionsOrHydration, + includesOnlyNonUrgentLanes, includesBlockingLane, includesExpiredLane, getNextLanes, @@ -1110,7 +1110,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootSuspendedWithDelay: { markRootSuspended(root, lanes); - if (includesOnlyTransitionsOrHydration(lanes)) { + if (includesOnlyNonUrgentLanes(lanes)) { // This is a transition, so we should exit without committing a // placeholder and without scheduling a timeout. Delay indefinitely // until we receive more data. From e6734cdcde30dba460b7b0b7227ef811443f4f03 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 1 Apr 2022 02:04:27 +0100 Subject: [PATCH 4/5] Moar tests --- .../src/__tests__/ReactDOMFizzServer-test.js | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index a06d7c5d17c53..8e4719e0b170b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2220,6 +2220,193 @@ describe('ReactDOMFizzServer', () => { }, ); + // @gate experimental + it('does not recreate the fallback if server errors and hydration suspends', async () => { + let isClient = false; + + function Child() { + if (isClient) { + readText('Yay!'); + } else { + throw Error('Oops.'); + } + Scheduler.unstable_yieldValue('Yay!'); + return 'Yay!'; + } + + const fallbackRef = React.createRef(); + function App() { + return ( +
+ Loading...

}> + + + +
+
+ ); + } + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + { + onError(error) { + Scheduler.unstable_yieldValue('[s!] ' + error.message); + }, + }, + ); + pipe(writable); + }); + expect(Scheduler).toHaveYielded(['[s!] Oops.']); + + // The server could not complete this boundary, so we'll retry on the client. + const serverFallback = container.getElementsByTagName('p')[0]; + expect(serverFallback.innerHTML).toBe('Loading...'); + + // Hydrate the tree. This will suspend. + isClient = true; + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue('[c!] ' + error.message); + }, + }); + // This should not report any errors yet. + expect(Scheduler).toFlushAndYield([]); + expect(getVisibleChildren(container)).toEqual( +
+

Loading...

+
, + ); + + // Normally, hydrating after server error would force a clean client render. + // However, it suspended so at best we'd only get the same fallback anyway. + // We don't want to recreate the same fallback in the DOM again because + // that's extra work and would restart animations etc. Check we don't do that. + const clientFallback = container.getElementsByTagName('p')[0]; + expect(serverFallback).toBe(clientFallback); + + // When we're able to fully hydrate, we expect a clean client render. + await act(async () => { + resolveText('Yay!'); + }); + expect(Scheduler).toFlushAndYield([ + 'Yay!', + '[c!] The server could not finish this Suspense boundary, ' + + 'likely due to an error during server rendering. ' + + 'Switched to client rendering.', + ]); + expect(getVisibleChildren(container)).toEqual( +
+ Yay! +
, + ); + }); + + // @gate experimental + it( + 'recreates the fallback if server errors and hydration suspends but ' + + 'client receives new props', + async () => { + let isClient = false; + + function Child() { + const value = 'Yay!'; + if (isClient) { + readText(value); + } else { + throw Error('Oops.'); + } + Scheduler.unstable_yieldValue(value); + return value; + } + + const fallbackRef = React.createRef(); + function App({fallbackText}) { + return ( +
+ {fallbackText}

}> + + + +
+
+ ); + } + + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + { + onError(error) { + Scheduler.unstable_yieldValue('[s!] ' + error.message); + }, + }, + ); + pipe(writable); + }); + expect(Scheduler).toHaveYielded(['[s!] Oops.']); + + const serverFallback = container.getElementsByTagName('p')[0]; + expect(serverFallback.innerHTML).toBe('Loading...'); + + // Hydrate the tree. This will suspend. + isClient = true; + const root = ReactDOMClient.hydrateRoot( + container, + , + { + onRecoverableError(error) { + Scheduler.unstable_yieldValue('[c!] ' + error.message); + }, + }, + ); + // This should not report any errors yet. + expect(Scheduler).toFlushAndYield([]); + expect(getVisibleChildren(container)).toEqual( +
+

Loading...

+
, + ); + + // Normally, hydration after server error would force a clean client render. + // However, that suspended so at best we'd only get a fallback anyway. + // We don't want to replace a fallback with the same fallback because + // that's extra work and would restart animations etc. Verify we don't do that. + const clientFallback1 = container.getElementsByTagName('p')[0]; + expect(serverFallback).toBe(clientFallback1); + + // However, an update may have changed the fallback props. In that case we have to + // actually force it to re-render on the client and throw away the server one. + root.render(); + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + expect(Scheduler).toHaveYielded([ + '[c!] The server could not finish this Suspense boundary, ' + + 'likely due to an error during server rendering. ' + + 'Switched to client rendering.', + ]); + expect(getVisibleChildren(container)).toEqual( +
+

More loading...

+
, + ); + // This should be a clean render without reusing DOM. + const clientFallback2 = container.getElementsByTagName('p')[0]; + expect(clientFallback2).not.toBe(clientFallback1); + + // Verify we can still do a clean content render after. + await act(async () => { + resolveText('Yay!'); + }); + expect(Scheduler).toFlushAndYield(['Yay!']); + expect(getVisibleChildren(container)).toEqual( +
+ Yay! +
, + ); + }, + ); + // @gate experimental it( 'errors during hydration force a client render at the nearest Suspense ' + From 6cb9f184c1246a29a0c5db00a5db6647c13dbdab Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 1 Apr 2022 02:38:51 +0100 Subject: [PATCH 5/5] Add test for transitions --- .../src/__tests__/ReactDOMFizzServer-test.js | 107 ++++++++++++++++-- 1 file changed, 100 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 8e4719e0b170b..295e99b26a691 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2247,14 +2247,11 @@ describe('ReactDOMFizzServer', () => { ); } await act(async () => { - const {pipe} = ReactDOMFizzServer.renderToPipeableStream( - , - { - onError(error) { - Scheduler.unstable_yieldValue('[s!] ' + error.message); - }, + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(, { + onError(error) { + Scheduler.unstable_yieldValue('[s!] ' + error.message); }, - ); + }); pipe(writable); }); expect(Scheduler).toHaveYielded(['[s!] Oops.']); @@ -2302,6 +2299,102 @@ describe('ReactDOMFizzServer', () => { ); }); + // @gate experimental + it( + 'does not recreate the fallback if server errors and hydration suspends ' + + 'and root receives a transition', + async () => { + let isClient = false; + + function Child({color}) { + if (isClient) { + readText('Yay!'); + } else { + throw Error('Oops.'); + } + Scheduler.unstable_yieldValue('Yay! (' + color + ')'); + return 'Yay! (' + color + ')'; + } + + const fallbackRef = React.createRef(); + function App({color}) { + return ( +
+ Loading...

}> + + + +
+
+ ); + } + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + { + onError(error) { + Scheduler.unstable_yieldValue('[s!] ' + error.message); + }, + }, + ); + pipe(writable); + }); + expect(Scheduler).toHaveYielded(['[s!] Oops.']); + + // The server could not complete this boundary, so we'll retry on the client. + const serverFallback = container.getElementsByTagName('p')[0]; + expect(serverFallback.innerHTML).toBe('Loading...'); + + // Hydrate the tree. This will suspend. + isClient = true; + const root = ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue('[c!] ' + error.message); + }, + }); + // This should not report any errors yet. + expect(Scheduler).toFlushAndYield([]); + expect(getVisibleChildren(container)).toEqual( +
+

Loading...

+
, + ); + + // Normally, hydrating after server error would force a clean client render. + // However, it suspended so at best we'd only get the same fallback anyway. + // We don't want to recreate the same fallback in the DOM again because + // that's extra work and would restart animations etc. Check we don't do that. + const clientFallback = container.getElementsByTagName('p')[0]; + expect(serverFallback).toBe(clientFallback); + + // Transition updates shouldn't recreate the fallback either. + React.startTransition(() => { + root.render(); + }); + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + const clientFallback2 = container.getElementsByTagName('p')[0]; + expect(clientFallback2).toBe(serverFallback); + + // When we're able to fully hydrate, we expect a clean client render. + await act(async () => { + resolveText('Yay!'); + }); + expect(Scheduler).toFlushAndYield([ + 'Yay! (red)', + '[c!] The server could not finish this Suspense boundary, ' + + 'likely due to an error during server rendering. ' + + 'Switched to client rendering.', + 'Yay! (blue)', + ]); + expect(getVisibleChildren(container)).toEqual( +
+ Yay! (blue) +
, + ); + }, + ); + // @gate experimental it( 'recreates the fallback if server errors and hydration suspends but ' +