Persisted state gets thrown away if any state is set in useEffect

[churichard]

I'm having a strange bug where my persisted state is getting overridden.

I'm using Next.js, Zustand with the persist middleware, and localForage using async IndexedDB. I have default values set in my store, but when my app first runs I want to use some different default values in a useEffect depending on whether the user is using a mobile device or not.

According to my mental model of Zustand, what should happen is that the useEffect runs before or after the persisted state is loaded. If it runs before, the persisted state should get merged in with priority. If it runs after, the persisted state should be merged in with the properties in useEffect having priority. What I'd actually like to happen is for the useEffect to get run before, but it doesn't matter for the purposes of this issue.

Instead, what seems to be happening is that all of my persisted properties get thrown away, and I get the default values along with the values I set in the useEffect. This doesn't make sense to me, and it seems like a bug. My persisted properties should always stay persisted if I haven't explicitly overridden them, which in this case I haven't -- all I've overridden are some other properties.

If I comment out the useEffect, then it works as you would expect: the persisted state stays persisted.

Hopefully this makes sense. If you'd like to take a look at my code to get a better idea of what I'm talking about, the store is here and the code that runs in the useEffect is here.

Edit:
Minimal reproducible case: https://github.com/churichard/zustand-persist-bug

Click the button to increase the number of bears (which is stored in IndexedDB). Once you refresh the page, the bears resets to 0.

Comment out the useEffect to see it work fine and for the number of bears persist.

[dai-shi]

Is this an issue only related with SSR? (In other words, is it reproducible with a client-only example?)
Does #182 seem related?

[churichard]

Thanks for the fast response!

The issue does not seem to be with SSR. I just spun up a quick reproducible case with create-react-app and the issue is still there.

Here's the repo: https://github.com/churichard/zustand-persist-bug

Click the button to increase the number of bears (which is stored in IndexedDB). Once you refresh the page, the bears resets to 0.

Comment out the useEffect to see it work fine and for the number of bears persist.

[dai-shi]

Thanks for the reproduction! This should help.
@AnatoleLucet Can you take a look?

[AnatoleLucet]

Not sure how this should be fixed, or even if it should be.
As you can see here and here, the persisted fields fully override the initial ones.

Changing the current behavior to something "smarter" might cause more trouble than leaving it that way.
Also keep in mind that because you set the state before hydration, the storage is set at page load with bears: 0. Might be tough to find a one-size-fits-all solution...
@dai-shi any thoughts?

For now, you should be able to fix it by setting the state once the store has been hydrated. You can do that with something like this: #346 (comment)

useEffect(() => {
  if (hasHydrated) {
    increaseBirds();
  }
}, [increaseBirds, hasHydrated]);

[AnatoleLucet]

Or maybe we delay the set()s until hydration happened 🤔
Might need to add an option to disable that just in case.

[dai-shi]

I think I understand set state is invoked before hydration, but why is the state not overwritten by the storage value?

[AnatoleLucet]

The useEffect is called before we get the storage's data.

First log is in the useEffect and the second in the storage.getItem:

[churichard]

If I understand correctly, this is happening because:

1. The default data is loaded: {bears: 0, birds: 0}
2. The useEffect runs, setting the birds property to 1
3. The state gets persisted into IndexedDB because of this set, and it persists the default value of bears (which is 0)
4. Then, when we read the persisted data afterwards, we're actually reading the default value of bears (which is 0) instead of whatever was persisted there before

Would it be possible to not persist any new data when the previous persisted data has not be read yet? If we can do that, then the useEffect would run, setting birds to 1, and then we wouldn't persist the default value of bears to IndexedDB, allowing the true persisted value of bears to merge in.

[AnatoleLucet]

Correct!

Would it be possible to not persist any new data when the previous persisted data has not be read yet?

Yeah, that's what I meant with this comment #562 (comment)

[churichard]

I think what you said was slightly different. What I mean is for the initial state to actually be updated in the useEffect, but for that update to not be persisted into IndexedDB. Then, when IndexedDB is read later, it will override both the initial state and whatever happened in the useEffect.

My use case is that I want to actually dynamically set the initial state. So there's some properties that can't be determined at build time (like whether the user is on a mobile device), and I want to change the default values based on that. But I want whatever is persisted in IndexedDB to always override whatever state was set by default or in the useEffect.

If that's actually what you meant, sorry 😅. I interpreted it as actually not setting any state at all until IndexedDB is read, which would mean that the useEffect would override the persisted properties in IndexedDB.

[dai-shi]

I see it overwrites data in the storage, before reading.

How does this hack fix, or not?

let initialized = false;
const storage = {
  getItem: async (name) => {
    const item = await localforage.getItem(name);
    initialized = true;
    return item;
  },
  setItem: async (name, value) => {
    if (!initialized) {
      console.warn('not ready yet');
      return;
    }
    await localforage.setItem(name, value);
  }
};

[AnatoleLucet]

I think what you said was slightly different. What I mean is for the initial state to actually be updated in the useEffect, but for that update to not be persisted into IndexedDB. Then, when IndexedDB is read later, it will override both the initial state and whatever happened in the useEffect.

Indeed I misunderstood 😄

This might be a better and more predictable default.
And if someone doesn't want their values to be overridden, they can still work around that with the merge option.

How does this hack fix, or not?

Should work. Though, if there's no set() afterward, the data won't be stored.

[churichard]

This might be a better and more predictable default.
And if someone doesn't want their values to be overridden, they can still work around that with the merge option.

I agree! There are already good options for overriding what's been persisted, but no good options for setting state before the persisted data is loaded. Plus, I think this is intuitively how people would think it would work if the useEffect is running first.

How does this hack fix, or not?

let initialized = false;
const storage = {
  getItem: async (name) => {
    const item = await localforage.getItem(name);
    initialized = true;
    return item;
  },
  setItem: async (name, value) => {
    if (!initialized) {
      console.warn('not ready yet');
      return;
    }
    await localforage.setItem(name, value);
  }
};

I just tried this out, but setting the state in useEffect doesn't seem to work if there is no persisted data (e.g. on first load). It just uses the default values.

For now, you should be able to fix it by setting the state once the store has been hydrated. You can do that with something like this: #346 (comment)

useEffect(() => {
  if (hasHydrated) {
    increaseBirds();
  }
}, [increaseBirds, hasHydrated]);

For now, I'm using this workaround. It's not perfect because it runs in the opposite order of which I want, so I need to use some heuristics to determine whether I should be keeping or throwing away the hydrated state. But it's a decent temporary solution 😄

[dai-shi]

@AnatoleLucet Should we close this issue or keep it? (Can you check all other issues for middleware/persist too?)

[dai-shi]

Closing as a workaround is provided.

[117]

Closing as a workaround is provided.

the workaround is brittle