Await storage setItem in persist middleware

[sumartoyo]

setItem call in persist middleware is not await-ed. But some storage may do the setItem asynchronously.

Code from middleware.ts

  const setItem = async () => {
    const state = { ...get() }

    if (whitelist) {
      Object.keys(state).forEach((key) => {
        !whitelist.includes(key) && delete state[key]
      })
    }
    if (blacklist) {
      blacklist.forEach((key) => delete state[key])
    }

    storage?.setItem(name, await serialize({ state, version }))
  }

Is it intentional? If it is then how to catch error in case an exception is thrown when calling setItem?

[AnatoleLucet]

I don't see why we'd need to await setItem nor catch an error related to it. Can you give an example?

[AnatoleLucet]

There is a possibility of underlying storage error.

Sure, but still, why should this middleware catch this error?

For example, when logging out a user, I would await it to make sure the persisted login-related state is cleared. Generally, it is to make sure that the state is persisted.

So something similar to onRehydrateStorage (but for setters) might be what you need for this?

[sumartoyo]

Oh sorry my bad. I didn't mean this middleware should catch the error. What I meant is the error should be bubbled up to the caller.

This is currently what will happen.

export const useStore = create(persist(
  (set, get) => ({
    fishes: 0,
    addAFish: async () => {
      try {
        await set({ fish: get().fish + 1 })
      } catch (err) {
        // error of storage.setItem() will never get here because it runs asynchronously, not awaited
      }
    }
  }),
  {
    name: "food-storage",
    getStorage: () => AsyncStorage,
  }
))

So something similar to onRehydrateStorage (but for setters) might be what you need for this?

Yes it will suit my need. Another way we can do it is by replacing middleware.ts#L219 with
return storage?.setItem(name, await serialize({ state, version }))
and middleware.ts#L226 with
return setItem()
In that way, the error from storage.setItem() will be "bubbled up" (I don't know a better term for it) to the caller.

But before going to code changes, I want to ask first if is it intentionally implemented that way? I mean maybe there are reasons why the error is not bubbled up to the caller that I miss here?

[AnatoleLucet]

Indeed, I didn't think about that.

Another way we can do it is by replacing middleware.ts#L219 with
return storage?.setItem(name, await serialize({ state, version }))
and middleware.ts#L226 with
return setItem()

You're right, it'd be easier to use.

But before going to code changes, I want to ask first if is it intentionally implemented that way?

No, it's not really intentional. I just didn't think of this use case.

[sumartoyo]

I see. So we can fix it if it is not intentional.

[AnatoleLucet]

Sure, feel free to open a PR if you want 🙂