From 02e1722f1858c0dcd1c3c66b626dcdfce9ba6e3d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 25 Apr 2022 15:37:45 +0100 Subject: [PATCH 1/2] Revert #24236 (Don't recreate the same fallback on the client if hydrating suspends) --- .../src/__tests__/ReactDOMFizzServer-test.js | 590 +++++++++--------- .../__tests__/ReactDOMHydrationDiff-test.js | 18 +- ...DOMServerPartialHydration-test.internal.js | 5 +- .../src/ReactFiberBeginWork.new.js | 1 - .../src/ReactFiberBeginWork.old.js | 1 - .../src/ReactFiberLane.new.js | 3 + .../src/ReactFiberLane.old.js | 3 + .../src/ReactFiberWorkLoop.new.js | 4 +- .../src/ReactFiberWorkLoop.old.js | 4 +- 9 files changed, 322 insertions(+), 307 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index bf5388597571e..c9c50bf57b492 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -869,16 +869,15 @@ describe('ReactDOMFizzServer', () => { }); // We still can't render it on the client. - expect(Scheduler).toFlushAndYield([]); - expect(getVisibleChildren(container)).toEqual(
Loading...
); - - // We now resolve it on the client. - resolveText('Hello'); - expect(Scheduler).toFlushAndYield([ 'The server could not finish this Suspense boundary, likely due to an ' + 'error during server rendering. Switched to client rendering.', ]); + expect(getVisibleChildren(container)).toEqual(
Loading...
); + + // We now resolve it on the client. + resolveText('Hello'); + Scheduler.unstable_flushAll(); // The client rendered HTML is now in place. expect(getVisibleChildren(container)).toEqual( @@ -2189,285 +2188,288 @@ 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( - '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 ' + - '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! -
, - ); - }, - ); + // Disabled because of a WWW late mutations regression. + // We may want to re-enable this if we figure out why. + + // // @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( + // '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 ' + + // '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( @@ -2542,12 +2544,17 @@ describe('ReactDOMFizzServer', () => { }, }); - // An error happened but instead of surfacing it to the UI, we suspended. - expect(Scheduler).toFlushAndYield([]); + // 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.', + ]); expect(getVisibleChildren(container)).toEqual(
- Yay! + Loading...
, ); @@ -2555,12 +2562,7 @@ describe('ReactDOMFizzServer', () => { await act(async () => { resolveText('Yay!'); }); - expect(Scheduler).toFlushAndYield([ - 'Yay!', - 'Hydration error', - 'There was an error while hydrating this Suspense boundary. Switched ' + - 'to client rendering.', - ]); + expect(Scheduler).toFlushAndYield(['Yay!']); 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 9237493c6dcec..f6b5a0b428a7d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js @@ -792,7 +792,7 @@ describe('ReactDOMServerHydration', () => { }); // @gate __DEV__ - it('does not warn when client renders an extra node inside Suspense fallback', () => { + it('warns when client renders an extra node inside Suspense fallback', () => { function Mismatch({isClient}) { return (
@@ -809,12 +809,15 @@ describe('ReactDOMServerHydration', () => {
); } - // There is no error because we don't actually hydrate fallbacks. - expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`Array []`); + 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.]", + ] + `); }); // @gate __DEV__ - it('does not warn when server renders an extra node inside Suspense fallback', () => { + it('warns when server renders an extra node inside Suspense fallback', () => { function Mismatch({isClient}) { return (
@@ -831,8 +834,11 @@ describe('ReactDOMServerHydration', () => {
); } - // There is no error because we don't actually hydrate fallbacks. - expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`Array []`); + 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.]", + ] + `); }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index fe9d8dd453cdb..e4d33645820e9 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -2115,7 +2115,10 @@ describe('ReactDOMServerPartialHydration', () => { }); suspend = true; - expect(Scheduler).toFlushAndYield([]); + expect(Scheduler).toFlushAndYield([ + 'The server could not finish this Suspense boundary, likely due to ' + + 'an error during server rendering. Switched to client rendering.', + ]); // 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 0764f16c89a0a..1538e32fca272 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2270,7 +2270,6 @@ 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 6d6e15e24af61..c50ca6357a5d9 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -2270,7 +2270,6 @@ 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 aa3360bd3c2b9..708c9318b3a98 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -459,6 +459,9 @@ export function includesOnlyNonUrgentLanes(lanes: Lanes) { const UrgentLanes = SyncLane | InputContinuousLane | DefaultLane; return (lanes & UrgentLanes) === NoLanes; } +export function includesOnlyTransitions(lanes: Lanes) { + return (lanes & TransitionLanes) === lanes; +} export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { if ( diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index a65a922d99b9a..e71aa5575abb6 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -459,6 +459,9 @@ export function includesOnlyNonUrgentLanes(lanes: Lanes) { const UrgentLanes = SyncLane | InputContinuousLane | DefaultLane; return (lanes & UrgentLanes) === NoLanes; } +export function includesOnlyTransitions(lanes: Lanes) { + return (lanes & TransitionLanes) === lanes; +} export function includesBlockingLane(root: FiberRoot, lanes: Lanes) { if ( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index aa20335cbec91..89c880d868554 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -131,7 +131,7 @@ import { pickArbitraryLane, includesNonIdleWork, includesOnlyRetries, - includesOnlyNonUrgentLanes, + includesOnlyTransitions, includesBlockingLane, includesExpiredLane, getNextLanes, @@ -1150,7 +1150,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootSuspendedWithDelay: { markRootSuspended(root, lanes); - if (includesOnlyNonUrgentLanes(lanes)) { + if (includesOnlyTransitions(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 e2c3a76c7fc24..2f5dbe8530ce5 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -131,7 +131,7 @@ import { pickArbitraryLane, includesNonIdleWork, includesOnlyRetries, - includesOnlyNonUrgentLanes, + includesOnlyTransitions, includesBlockingLane, includesExpiredLane, getNextLanes, @@ -1150,7 +1150,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { case RootSuspendedWithDelay: { markRootSuspended(root, lanes); - if (includesOnlyNonUrgentLanes(lanes)) { + if (includesOnlyTransitions(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 d7a9ef36afcb80ff56b994a810f9852c50be5d52 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 25 Apr 2022 16:05:11 +0100 Subject: [PATCH 2/2] Use @gate FIXME --- .../src/__tests__/ReactDOMFizzServer-test.js | 564 +++++++++--------- 1 file changed, 285 insertions(+), 279 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index c9c50bf57b492..72ed9f8775dab 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2190,286 +2190,292 @@ describe('ReactDOMFizzServer', () => { // Disabled because of a WWW late mutations regression. // We may want to re-enable this if we figure out why. + // @gate experimental + // @gate FIXME + it('does not recreate the fallback if server errors and hydration suspends', async () => { + let isClient = false; - // // @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( - // '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 ' + - // '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! - //
, - // ); - // }, - // ); + 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! +
, + ); + }); + + // Disabled because of a WWW late mutations regression. + // We may want to re-enable this if we figure out why. + // @gate experimental + // @gate FIXME + 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) +
, + ); + }, + ); + + // Disabled because of a WWW late mutations regression. + // We may want to re-enable this if we figure out why. + // @gate experimental + // @gate FIXME + 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(