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

feat: getInitialState #2277

Merged
merged 15 commits into from
Jan 20, 2024
Merged

feat: getInitialState #2277

merged 15 commits into from
Jan 20, 2024

Conversation

TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented Dec 29, 2023

This PR adds a new getInitialState method to the store api, which will always return the initial state that was used to create the store. The persist middleware also implements this method because it alters the initial state when restoring from a storage.

In React, this new method can then be used for getServerSnaphot in useSyncExternalStore to avoid avoid hydration mismatches when the state changes between the SSR and the first CSR. This could happen before when using the persist middleware, but can also happen whenever the store state was changed on the client only before completing the first CSR.

Related Issues or Discussions

Fixes #1174

Summary

Check List

  • yarn run prettier for formatting code and docs

serverState will be used by react on the first client render; this should avoid hydration mismatches when combined with the persist middleware, which can change the state between the SSR and the first CSR
Copy link

vercel bot commented Dec 29, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
zustand-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 20, 2024 0:02am

Copy link

codesandbox-ci bot commented Dec 29, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5aaf3a8:

Sandbox Source
React Configuration
React TypeScript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration
Next.js Configuration
@pavlobu/zustand demo Configuration

src/react.ts Outdated Show resolved Hide resolved
@TkDodo
Copy link
Collaborator Author

TkDodo commented Dec 29, 2023

okay this doesn't work - here's the reproduction depending on the preview build:

https://stackblitz.com/edit/stackblitz-starters-lxg5yy?file=app%2Fpage.tsx

I've traced it down to when we're calling

const initialState = api.getState()

the state already contains the hydrated one from localstorage. So the question is now - where do I best get access to capturing the actual initial state that was passed to create ?

debugging a bit more, it seems that the persist middleware needs to yield both the values from localstorage as well as the default value? I don't see createStore getting access to it when chaining createStore(persist(...)) because persist basically "swallows" it ...

@TkDodo TkDodo marked this pull request as draft December 29, 2023 11:20
src/react.ts Outdated Show resolved Hide resolved
@dai-shi
Copy link
Member

dai-shi commented Dec 29, 2023

the state already contains the hydrated one from localstorage.

Yeah, middleware can modify state during initialization.
I was thinking something like this:

const initialState = ...
const useFooStore = create(persist((set) => ({
  ...initialState
}), { ... }))
useFooStore.getServerState = () => initialState

@dai-shi
Copy link
Member

dai-shi commented Dec 29, 2023

Maybe, this would be more developer friendly:

import { create } from 'zustand'

const useFooStore = create(
  (set) => ({ ... }),
  () => ({ ... }), // for server state
)

@TkDodo
Copy link
Collaborator Author

TkDodo commented Dec 29, 2023

But both approaches would be something that must be done in user-land, right?

if the persist middleware would somehow yield the serverState (or initialState), we could use that in the react adapter. I'm thinking that this should work ootb because without getServerSnapshot implemented, any state change between ssr and csr would trigger a hydration warning - this is independent of the persist middleware

@dai-shi
Copy link
Member

dai-shi commented Dec 29, 2023

Yeah, so if we need ootb solution, vanilla store should know about react convention. It's reasonable as most Zustand users are React users.

If that's the case, we can simply define optional getServerState in api in vanilla.ts. It's up to middleware how to implement it.

@dai-shi
Copy link
Member

dai-shi commented Dec 29, 2023

we can simply define optional getServerState in api in vanilla.ts.

Let me add a commit.

this avoids hydration errors when state is restored from e.g. localstorage synchronously
@TkDodo
Copy link
Collaborator Author

TkDodo commented Dec 29, 2023

thanks @dai-shi 🙏

I've added one more commit where the persist middleware actually sets getServerState: 6d046fe

However, I still think we should bring back setting getServerState in the react createImpl in case getServerState hasn't been set already to avoid hydration errors from other updates that could happen to the store in-between (that are unrelated to persist middleware). What do you think?

src/middleware/persist.ts Outdated Show resolved Hide resolved
this avoids hydration mismatches when updates happen to the store state between ssr and csr
src/react.ts Outdated Show resolved Hide resolved
@TkDodo
Copy link
Collaborator Author

TkDodo commented Dec 29, 2023

I'm still getting hydration errors (latest stackblitz fork), but I don't really understand it.

error is:

Warning: Text content did not match. Server: "hydration-default" Client: "hydration-local"

debugging this at the useSyncExternalStore call however leads to this:

Screenshot 2023-12-29 at 18 32 04

getServerState returns the correct server value that we pass to getServerSnapshot, which means it should be used by react for the server render and the first client render.

Not really sure how this can still show a hydration error 🤔

if we default to `api.getState`, we will always read the client snapshot if there is no selector passed. An identity function returns its argument, which is either the snapshot (api.getState) or the server snapshot (api.getServerState)
@TkDodo
Copy link
Collaborator Author

TkDodo commented Dec 30, 2023

found out what the issue is. It's because we're using useSyncExternalStoreWithSelector, and the default selector is api.getState, which will always yield the client state:

selector: (state: TState) => StateSlice = api.getState as any,

I think the selector should just be an identity function, as it gets the snapshot passed (which is either the result from api.getState or api.getServerState.

I think this should fix it: a985abe 🤞

@TkDodo
Copy link
Collaborator Author

TkDodo commented Dec 30, 2023

yep, here is a working fork without hydration errors 🎉

https://stackblitz.com/edit/stackblitz-starters-y74eij?file=app%2Fpage.tsx

@dai-shi
Copy link
Member

dai-shi commented Dec 30, 2023

I think this should fix it: a985abe 🤞

Nice. It's historical code and I have been wondering if we should use identity function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants