From 4cd7065665ea2cf33c306265c8d817904bb401ca Mon Sep 17 00:00:00 2001 From: Sophie Alpert Date: Fri, 12 May 2023 14:18:03 -0700 Subject: [PATCH] Fix uSES hydration in strict mode (#26791) Previously, we'd call and use getSnapshot on the second render resulting in `Warning: Text content did not match. Server: "Nay!" Client: "Yay!"` and then `Error: Text content does not match server-rendered HTML.`. Fixes #26095. Closes #26113. Closes #25650. --------- Co-authored-by: eps1lon --- .../src/__tests__/ReactDOMFizzServer-test.js | 94 +++++++++++++++++++ .../react-reconciler/src/ReactFiberHooks.js | 40 +++++--- 2 files changed, 120 insertions(+), 14 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index f4efd7959e511..2bf48eae39b18 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -20,6 +20,7 @@ let JSDOM; let Stream; let Scheduler; let React; +let ReactDOM; let ReactDOMClient; let ReactDOMFizzServer; let Suspense; @@ -73,6 +74,7 @@ describe('ReactDOMFizzServer', () => { Scheduler = require('scheduler'); React = require('react'); + ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); ReactDOMFizzServer = require('react-dom/server'); Stream = require('stream'); @@ -2507,6 +2509,98 @@ describe('ReactDOMFizzServer', () => { }, ); + it('can hydrate uSES in StrictMode with different client and server snapshot (sync)', async () => { + function subscribe() { + return () => {}; + } + function getClientSnapshot() { + return 'Yay!'; + } + function getServerSnapshot() { + return 'Nay!'; + } + + function App() { + const value = useSyncExternalStore( + subscribe, + getClientSnapshot, + getServerSnapshot, + ); + Scheduler.log(value); + + return value; + } + + const element = ( + + + + ); + + await act(async () => { + const {pipe} = renderToPipeableStream(element); + pipe(writable); + }); + + assertLog(['Nay!']); + expect(getVisibleChildren(container)).toEqual('Nay!'); + + await clientAct(() => { + ReactDOM.flushSync(() => { + ReactDOMClient.hydrateRoot(container, element); + }); + }); + + expect(getVisibleChildren(container)).toEqual('Yay!'); + assertLog(['Nay!', 'Yay!']); + }); + + it('can hydrate uSES in StrictMode with different client and server snapshot (concurrent)', async () => { + function subscribe() { + return () => {}; + } + function getClientSnapshot() { + return 'Yay!'; + } + function getServerSnapshot() { + return 'Nay!'; + } + + function App() { + const value = useSyncExternalStore( + subscribe, + getClientSnapshot, + getServerSnapshot, + ); + Scheduler.log(value); + + return value; + } + + const element = ( + + + + ); + + await act(async () => { + const {pipe} = renderToPipeableStream(element); + pipe(writable); + }); + + assertLog(['Nay!']); + expect(getVisibleChildren(container)).toEqual('Nay!'); + + await clientAct(() => { + React.startTransition(() => { + ReactDOMClient.hydrateRoot(container, element); + }); + }); + + expect(getVisibleChildren(container)).toEqual('Yay!'); + assertLog(['Nay!', 'Yay!']); + }); + it( 'errors during hydration force a client render at the nearest Suspense ' + 'boundary, and during the client render it recovers', diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 30440c8b69a60..19ad56d0f8cae 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1776,8 +1776,6 @@ function mountSyncExternalStore( // clean-up function, and we track the deps correctly, we can call pushEffect // directly, without storing any additional state. For the same reason, we // don't need to set a static flag, either. - // TODO: We can move this to the passive phase once we add a pre-commit - // consistency check. See the next comment. fiber.flags |= PassiveEffect; pushEffect( HookHasEffect | HookPassive, @@ -1799,15 +1797,28 @@ function updateSyncExternalStore( // Read the current snapshot from the store on every render. This breaks the // normal rules of React, and only works because store updates are // always synchronous. - const nextSnapshot = getSnapshot(); - if (__DEV__) { - if (!didWarnUncachedGetSnapshot) { - const cachedSnapshot = getSnapshot(); - if (!is(nextSnapshot, cachedSnapshot)) { - console.error( - 'The result of getSnapshot should be cached to avoid an infinite loop', - ); - didWarnUncachedGetSnapshot = true; + let nextSnapshot; + const isHydrating = getIsHydrating(); + if (isHydrating) { + // Needed for strict mode double render + if (getServerSnapshot === undefined) { + throw new Error( + 'Missing getServerSnapshot, which is required for ' + + 'server-rendered content. Will revert to client rendering.', + ); + } + nextSnapshot = getServerSnapshot(); + } else { + nextSnapshot = getSnapshot(); + if (__DEV__) { + if (!didWarnUncachedGetSnapshot) { + const cachedSnapshot = getSnapshot(); + if (!is(nextSnapshot, cachedSnapshot)) { + console.error( + 'The result of getSnapshot should be cached to avoid an infinite loop', + ); + didWarnUncachedGetSnapshot = true; + } } } } @@ -1830,7 +1841,7 @@ function updateSyncExternalStore( if ( inst.getSnapshot !== getSnapshot || snapshotChanged || - // Check if the susbcribe function changed. We can save some memory by + // Check if the subscribe function changed. We can save some memory by // checking whether we scheduled a subscription effect above. (workInProgressHook !== null && workInProgressHook.memoizedState.tag & HookHasEffect) @@ -1854,7 +1865,7 @@ function updateSyncExternalStore( ); } - if (!includesBlockingLane(root, renderLanes)) { + if (!isHydrating && !includesBlockingLane(root, renderLanes)) { pushStoreConsistencyCheck(fiber, getSnapshot, nextSnapshot); } } @@ -2217,7 +2228,8 @@ function updateEffectImpl( const effect: Effect = hook.memoizedState; const inst = effect.inst; - // currentHook is null when rerendering after a render phase state update. + // currentHook is null on initial mount when rerendering after a render phase + // state update or for strict mode. if (currentHook !== null) { if (nextDeps !== null) { const prevEffect: Effect = currentHook.memoizedState;