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

A swapped in store will not update reactively #721

Closed
zzmp opened this issue Dec 23, 2021 · 10 comments
Closed

A swapped in store will not update reactively #721

zzmp opened this issue Dec 23, 2021 · 10 comments

Comments

@zzmp
Copy link

zzmp commented Dec 23, 2021

I'm trying to use zustand for a project where a user can specify a zustand-backed provider. If the user switches their provider, any changes to the new providers store are not picked up by react. This is because any uses of the zustand hook useStore will not be updated to listen to the new provider, because the listener is set up once, with no deps.

As a contrived (untested) example:

<MyComponent storeBackedProvider={mock} />
function MyComponent({storeBackedProvider}) {
  const useStore = create(storeBackedProvider.store)
  const value = useStore()

  return <span>{value}</span>
}

In this case, if the mock is swapped out, MyComponent will no longer be reactive to value.


I propose depending on api when registering listeners to fix this.

@dai-shi
Copy link
Member

dai-shi commented Dec 23, 2021

Oh,,, I overlooked this. (Hm, why does react-hooks eslint rule warn this? Maybe, because it's in a create function.)

Can you also try if #550 fixes this issue?

@zzmp
Copy link
Author

zzmp commented Dec 23, 2021

#550 seems to resolve this issue.
Feel free to take the test from #722 and patch it onto #550 if you want to test against regressions.

@dai-shi
Copy link
Member

dai-shi commented Dec 23, 2021

@zzmp I'm very sorry. I didn't correctly understand what this is about, went ahead to #722, and thought it's fine.

But, this is not something we want to allow. It's like conditionally invoking hooks, which is basically violating the rule of hooks. We shouldn't dynamically create a hook in a component. It just happen to work because the useStore hook has the same implementation, but in general it's not guaranteed, and it looks like a bad practice.

zustand v3 has a workaround with zustand/context which make sure the store is created just once.

In your use case, what's easy is to remount the component:

<MyComponent key={unique_string_for_mock} storeBackedProvider={mock} />

I agree this is a non-trivial limitation and zustand v4 with #550 will provide a new API for this use case. So, please be patient.

My big apologies again for your work in #722 with my wrong understanding.

@dai-shi
Copy link
Member

dai-shi commented Dec 23, 2021

#550 seems to resolve this issue.

Interesting, but understandable because useSyncExternalStore will re-subscribe if api.subscrib changes.
In that sense, #722 is a right fix technically.
But, since I would like to promote the new api in v4, I want to avoid the dynamic creation pattern.

@dai-shi
Copy link
Member

dai-shi commented Dec 23, 2021

I want to avoid the dynamic creation pattern.

For someone interested, the way I see is this.

  // this is creating a hook dynamically in render
  const useStore = create(props.createState)
  const value = useStore()

  // which is technically the same as this (which looks like a bad practice)
  const value = props.condition ? useState(0) : useState(1)

@zzmp
Copy link
Author

zzmp commented Dec 29, 2021

@dai-shi if the behavior will change in #550, and #722 will introduce the new behavior early without any breaking changes to v3, would it be ok to merge #722 so that the new behavior can be used as part of v3 too?

We were hoping to use zustand because it lets you initialize stores outside of the React lifecycle, but this point - while it does look like an antipattern - removes that "feature" that we wanted out of zustand, by forcing us to initialize the store with the react component.

@dai-shi
Copy link
Member

dai-shi commented Dec 29, 2021

To my understanding #550 works by chance, and we don't guarantee the usage for the future. So even when #550 is merged, that's not a recommended usage.

Doesn't key={} solve your use case?

When v4 lands, we can use useStore(store), if you are keen to try it now, use https://www.npmjs.com/package/zustand/v/4.0.0-beta.0 , but there's no docs yet.

@zzmp
Copy link
Author

zzmp commented Dec 29, 2021

key={} does not solve my case, because the key should only be changed when the new store is ready, and that's exposed by hooks (which, in this case, are not being subscribed to).

I've switched to 4.0.0-beta.0, and it does solve my case, without any extra work. Do you know why it does so?

@dai-shi
Copy link
Member

dai-shi commented Dec 29, 2021

Because 4.0.0-beta.0 includes #550.

Note again that my recommendation is to use useStore(store).
The conditional hook cond ? useStoreA() : useStoreB() seems a bad practice, and I wouldn't recommend it even if it works.

@dai-shi
Copy link
Member

dai-shi commented Jan 11, 2022

Closing this.
Swapping stores is not currently supported in zustand, which is a bad practice with React Hooks.
v4 will provide a new API to support swapping stores.

@dai-shi dai-shi closed this as completed Jan 11, 2022
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 a pull request may close this issue.

2 participants