Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React 18: hydration mismatch when an external store is updated in an effect #22361

Closed
eps1lon opened this issue Sep 20, 2021 · 7 comments
Closed
Labels
React 18 Bug reports, questions, and general feedback about React 18 Type: Discussion

Comments

@eps1lon
Copy link
Collaborator

eps1lon commented Sep 20, 2021

React version: #22347

Steps To Reproduce

  1. Read store (using useSyncExternalStore) inside a Suspense boundary where no component suspended
  2. Read store (using useSyncExternalStore) outside of any Suspense boundary
  3. Update store outside of any Suspense boundary in an effect (useEffect)
<Store>
  <Suspense fallback={null}>
    <Demo>inside suspense has hydration mismatch</Demo>
    {/* When the fallback is actually used on the server, we don't get a hydration mismatch */}
    {/* <Suspender /> */}
  </Suspense>
  <Demo>outside suspense has no hydration mismatch</Demo>
  <UpdatesStore />
</Store>
);

Link to code example: https://codesandbox.io/s/react-18-updating-store-in-an-effect-during-mount-causes-hydration-mismatch-uses-m6lwm?file=/src/index.js

The current behavior

<Demo /> inside the Suspense boundary causes a hydration mismatch since it's hydrated with the value set during useEffect.

The expected behavior

No hydration mismatch.

Repro explainer

The repro is based on reduxjs/react-redux#1794 which is based on a usage from the mui.com docs.
The behavior this repro is implementing is reading a value from window.localStorage (e.g. settings) with a fallback on the server.

The store is a Redux store that is the same on Server and Client.
Reading from the store is implemented like so:

const ReduxStoreContext = createContext();

function useValueRedux() {
  const store = useContext(ReduxStoreContext);
  const selector = useCallback(() => store.getState().codeVariant, [store]);

  // The store is equivalent on Client and Server so we can just re-use `getSnapshot` for `getServerSnapshot`
  return useSyncExternalStore(store.subscribe, selector, selector);
}

The repro contains an implementation that uses React Context for the store which works as expected i.e. no hydration mismatches.

Context

I recently stumbled over

(In general, updates inside a passive effect are not encouraged.)

-- #22277

which makes it sound like this behavior is expected because the update is inside a passive effect. Though it's unclear what is meant by "not encourage". How would I render a default value on the Server and populate it with the actually desired value on the Client?

@eps1lon eps1lon added Type: Discussion React 18 Bug reports, questions, and general feedback about React 18 labels Sep 20, 2021
@acdlite
Copy link
Collaborator

acdlite commented Sep 20, 2021

This is the purpose of the getServerSnapshot argument. It looks like you're passing the same getServerSnapshot as you are the regular getSnapshot. From your CodeSandbox:

const selector = useCallback(() => store.getState().codeVariant, [store]);
return useSyncExternalStore(store.subscribe, selector, selector);

Instead, the third argument should return the value used by the server:

const getSnapshot = ()=> store.getState().codeVariant;
const getServerSnapshot = () => SERVER_STATE.codeVariant;
return useSyncExternalStore(store.subscribe, getSnapshot, getServerSnapshot);

(Btw there's no performance advantage to these being memoized since they return immutable values.)

How you implement SERVER_STATE will vary, but in the case of Redux, since it's an immutable store, you could do const SERVER_STATE = store.getState() right before you start rendering. Then serialize the state and send it down to the client.

@acdlite
Copy link
Collaborator

acdlite commented Sep 20, 2021

To explain how React uses this argument, it will call getServerSnapshot instead of getSnapshot both on the server and during hydration on the client. This way you can tell React exactly what value was used to render on the server and avoid mismatches.

If something does change on the client, like in an effect, React will first hydrate the affected subtree, then apply the update on top of that.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Sep 20, 2021

This is the purpose of the getServerSnapshot argument. It looks like you're passing the same getServerSnapshot as you are the regular getSnapshot.

Because the store is actually the same on client and server as far as I can tell. But I don't know if any component in the tree might update the store (in a concurrent compliant way).

Could you explain why my example works perfectly fine if I don't use Suspense? The server rendered and client rendered markup is the same with and without Suspense in my example. It just fails to hydrate once wrapped in Suspense.

I'll play around with a different example to see how React handles it.

But for me, the current uSES implementation breaks assumptions about composition. Now I have to tell every usage of the store that they can't update the store in an effect.

How you implement SERVER_STATE will vary, but in the case of Redux, since it's an immutable store, you could do const SERVER_STATE = store.getState() right before you start rendering. Then serialize the state and send it down to the client.

I don't understand how this is not what I'm already doing. It's just serialized in code instead of JSON since the store value is hardcoded:

const store = useMemo(() => {
  return createStore(
    (state = { value: "js" }, action) => {
      if (action.type === "VALUE_SET") {
        return { value: action.payload };
      }
      return state;
    },
    { value: "js" }
  );
}, []);

-- https://codesandbox.io/s/react-18-updating-store-in-an-effect-during-mount-causes-hydration-mismatch-uses-m6lwm?file=/src/index.js:1460-1750

@eps1lon
Copy link
Collaborator Author

eps1lon commented Sep 20, 2021

I think I get it now. Even though I know that the server store and initial client store are the same, I don't know if the client store is still unchanged once a particular reader is getting hydrated. So I need to make sure I read from the initial copy.

For my minimal example that's basically.

 function StoreRedux({ children }) {
   const store = useMemo(() => {
     return createStore(
       (state = { value: "js" }, action) => {
         if (action.type === "VALUE_SET") {
           return { ...state, value: action.payload };
         }
         return state;
       },
-      { value: "js" }
+      { value: "js", server: { value: "js" } }
     );
   }, []);


 function useValueRedux() {
   const store = useContext(ReduxStoreContext);
   const getSnapshot = useCallback(() => store.getState().value, [store]);
+  const getServerSnapshot = useCallback(() => store.getState().server.value, [
+    store
+  ]);

-  return useSyncExternalStore(store.subscribe, getSnapshot, getSnapshot);
+  return useSyncExternalStore(store.subscribe, getSnapshot, getServerSnapshot);
 }

https://codesandbox.io/s/react-18-updating-store-in-an-effect-during-mount-causes-hydration-mismatch-uses-fixed-k3gpr

Edit:
I wonder if this will be a common mistake where people just pass the same method to getServerSnapshot as to getSnapshot to prevent a crash on the server while inevitably causing hydration mismatches. So for me particular a warning for getSnapshot === getServerSnapshot would've been worthwile.

@acdlite
Copy link
Collaborator

acdlite commented Sep 20, 2021

Could you explain why my example works perfectly fine if I don't use Suspense? The server rendered and client rendered markup is the same with and without Suspense in my example. It just fails to hydrate once wrapped in Suspense.

It's because React breaks up the hydration using the Suspense boundaries. So with the Suspense boundary, the effect hydrates and fires before the inner tree, causing a mismatch. Without the boundary, everything hydrates and commits in the same batch.

@acdlite
Copy link
Collaborator

acdlite commented Sep 20, 2021

I wonder if this will be a common mistake where people just pass the same method to getServerSnapshot as to getSnapshot to prevent a crash on the server while inevitably causing hydration mismatches. So for me particular a warning for getSnapshot === getServerSnapshot would've been worthwile.

Yeah we could consider that though maybe documenting it is good enough.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Sep 20, 2021

Alright, thanks for clarifying 👍🏻 Closing since this an author error not library error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
React 18 Bug reports, questions, and general feedback about React 18 Type: Discussion
Projects
None yet
Development

No branches or pull requests

2 participants