Skip to content

Commit

Permalink
Fix uSES hydration in strict mode (#26791)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
sophiebits and eps1lon authored May 12, 2023
1 parent a389046 commit 4cd7065
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 14 deletions.
94 changes: 94 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ let JSDOM;
let Stream;
let Scheduler;
let React;
let ReactDOM;
let ReactDOMClient;
let ReactDOMFizzServer;
let Suspense;
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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 = (
<React.StrictMode>
<App />
</React.StrictMode>
);

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 = (
<React.StrictMode>
<App />
</React.StrictMode>
);

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',
Expand Down
40 changes: 26 additions & 14 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1776,8 +1776,6 @@ function mountSyncExternalStore<T>(
// 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,
Expand All @@ -1799,15 +1797,28 @@ function updateSyncExternalStore<T>(
// 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;
}
}
}
}
Expand All @@ -1830,7 +1841,7 @@ function updateSyncExternalStore<T>(
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)
Expand All @@ -1854,7 +1865,7 @@ function updateSyncExternalStore<T>(
);
}

if (!includesBlockingLane(root, renderLanes)) {
if (!isHydrating && !includesBlockingLane(root, renderLanes)) {
pushStoreConsistencyCheck(fiber, getSnapshot, nextSnapshot);
}
}
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 4cd7065

Please sign in to comment.