Mocking state broken in 4.2.0 (with AsyncStorage in React Native)

[swemail]

Summary

Hello and thank you for a great state library!

When upgrading to 4.2.0 and using persist, the suggested mock approach throws error TypeError: Cannot read properties of undefined (reading 'getItem')

Suggested mock code that breaks:

// when creating a store, we get its initial state, create a reset function and add it in the set
const create =
  () =>
  <S>(createState: StateCreator<S>) => {
    const store = actualCreate<S>(createState); // <-- breaks here
    const initialState = store.getState();
    storeResetFns.add(() => store.setState(initialState, true));
    return store;
  };

Our persist setup:

import create from 'zustand';
import {createJSONStorage, persist} from 'zustand/middleware';
import AsyncStorage from '@react-native-async-storage/async-storage';
...
const useAppState = create<State & Actions>()(
  persist(
    set => ({
      ...initialState,
      setIsColdStarted: isColdStarted => set({isColdStarted}),
      setSettings: settings =>
        set(state => ({settings: {...state.settings, ...settings}})),
      reset: () => set(initialState),
    }),
    {
      name: 'appState',
      storage: createJSONStorage(() => AsyncStorage), // <-- instead of deprecated getStorage: () => AsyncStorage
      partialize: ({settings}) => ({
        settings,
      }),
    },
  ),
);

[dai-shi]

Does it work with the deprecated getStorage?

[mgrahamspoton]

We get the same issue but we are not using persist

we get the following error when our vanilla.js file has:

const create = f =>
  f === undefined ? createImpl : createImpl(f)

// when creating a store, we get its initial state, create a reset function and add it in the set
const createImpl = (createState) => {
  const store = actualCreate(createState)
  const initialState = store.getState()
  storeResetFns.add(() => store.setState(initialState, true))
  return store
}

if we use:

const create = () => createState => {
    const store = actualCreate(createState);
    const initialState = store.getState();
    storeResetFns.add(() => store.setState(initialState, true));
    return store;
};

we get the following error:

[dai-shi]

We get the same issue but we are not using persist

Isn't it the problem only in 4.2.0? The error message seems different from OP.

How do you define actualCreate? Something seems wrong. If you need help, please open a new discussion.

[swemail]

Does it work with the deprecated getStorage?

Yes, with getStorage it works.

[dai-shi]

Does it work with the deprecated getStorage?

Yes, with getStorage it works.

Thanks for confirming. Not sure what's happening, but there must be something somewhere.
Can you or anyone dig into it a bit deeper?
Or, can create a small reproduction without testing context?

[MHC03]

Problem seems to be here: https://github.com/pmndrs/zustand/blob/main/src/middleware/persist.ts#L38.
Haven't analyzed the specifics, but str there seems to be an object already, so using str.toString() has done the trick for me for the tests. Would need further analysis, maybe you already know what might be wrong.

[MHC03]

Possible solution. Please check if this seems right for you:

if (str === null || Object.keys(str).length === 0) {
  return null;
}
return JSON.parse(str);

[dai-shi]

Thanks @MHC03 for the investigation!
The suggested solution doesn't seem right, unless it has a problem without mocking. There must be a better fix for mocking otherwise.
But, that indicates our newImpl is somewhat different from oldImpl. One thing I notice is that the newImpl (with createJSONStorage) assumes native Promise. If RN's AsyncStorage doesn't return a native Promise value, it may not work.

@MHC03 If you have chance, please keep investigating.

@swemail Can you confirm if 4.2.0 works with RN's AsyncStorage without mocking?

[MHC03]

For mocking the AsyncStorage I use the mock implementation of RN's AsyncStorage. After further investigation I found out, that str instanceof Promise in that case is false. My suggestion would be to directly return the promise instead of checking for a Promise with an if-statement.:

const str = (storage as StateStorage).getItem(name) ?? null
return Promise.resolve(str).then(parse)

Edit: @swemail How have you mocked your AsyncStorage? I followed this documentation: https://react-native-async-storage.github.io/async-storage/docs/advanced/jest/

[dai-shi]

return Promise.resolve(str).then(parse)

It isn't very nice for sync storage. That's what we put most effort in the old impl.

We could check if .then is a function, or str is string or null.

the newImpl (with createJSONStorage) assumes native Promise.

On another look, oldImpl also depends on instanceof Promise. So, I'm not sure how the old impl works in this case.

[swemail]

Thanks @MHC03 for the investigation! The suggested solution doesn't seem right, unless it has a problem without mocking. There must be a better fix for mocking otherwise. But, that indicates our newImpl is somewhat different from oldImpl. One thing I notice is that the newImpl (with createJSONStorage) assumes native Promise. If RN's AsyncStorage doesn't return a native Promise value, it may not work.

@MHC03 If you have chance, please keep investigating.

@swemail Can you confirm if 4.2.0 works with RN's AsyncStorage without mocking?

Yes it works perfectly without mocking

[swemail]

For mocking the AsyncStorage I use the mock implementation of RN's AsyncStorage. After further investigation I found out, that str instanceof Promise in that case is false. My suggestion would be to directly return the promise instead of checking for a Promise with an if-statement.:

const str = (storage as StateStorage).getItem(name) ?? null
return Promise.resolve(str).then(parse)

Edit: @swemail How have you mocked your AsyncStorage? I followed this documentation: https://react-native-async-storage.github.io/async-storage/docs/advanced/jest/

Yes, same way, using mocks directory approach

[MHC03]

Digged in further again and found out that somehow AsyncStorageMock.getItem is of type Promise { <pending> } and this does not comply with instanceof Promise (should be Promise { _x: 0, _y: 1, _z: undefined, _A: null }). I can't explain why it has this difference. So, overwriting the getItem attribute solved the issue for me...

AsyncStorageMock.getItem = jest.fn(async (key, callback) => await AsyncStorageMock.getItem(key, callback));

[dai-shi]

Digged in further again and found out that somehow AsyncStorageMock.getItem is of type Promise { <pending> } and this does not comply with instanceof Promise (should be Promise { _x: 0, _y: 1, _z: undefined, _A: null }). I can't explain why it has this difference.

Nice finding though. Can you try getStorage instead of storage option and see why it works even with instanceof Promise being false?

[meypod]

So I came here to report that some part of my app functionality breaks when upgrading 4.1.5 to 4.2.0 and using storage: createJSONStorage(() => zustandStorage),

but on 4.2.0, using the deprecated getStorage option fixes my issue.
what's my issue ? here is the snippet of my code, I'll explain more below it:

  useEffect(() => {
    const unsub = settings.subscribe((state, prevState) => {
      if (state.SELECTED_LOCALE !== prevState.SELECTED_LOCALE) {
        console.log(prevState.SELECTED_LOCALE, state.SELECTED_LOCALE);
        loadLocale(state.SELECTED_LOCALE);
        I18nManager.forceRTL(isRTL);
        // allow some time for forceRTL to work
        setTimeout(restart, 200);
      }
    });

    return unsub;
  });

I have a component with snippet above, where it has a drop down component that changes SELECTED_LOCALE
on storage option, snippet keeps firing, causing app to infinitely restart. this is odd because the console.log shown above, is always logging en ar, with ar being the new locale that dropdown selected. as if the drop down is always firing the change, even after restart, but clearly it's not !
on getStorage it's totally fine though, everything works as expected

I thought this issue is probably related so decided to add my own info here
I hope this helps to pin down the issue

[dai-shi]

@meypod Is your issue only with mocking? If it's an issue for non-mocking context, you might want to open a new discussion?

Thanks for chiming in anyway. It might be a hint for someone.

[meypod]

hmm, sorry I thought it's clear from my comment that it's not in mocking, but in actual app that its causing problem. I just thought maybe this issue and mine have the same root cause
but I guess a new issue might be better for it
edit: I see I have to open discussion before openning an issue, doing that ;)
edit2: #1516

[dai-shi]

Unlike what I expected, #1517 issue was related with toThenable, as @meypod have thought.

#1518 may fix this issue too. Can @swemail or @MHC03 confirm?

[MHC03]

@dai-shi Your fix solves the problem, but this makes all of this more complicated than I thought. The reason, why all of this works with toThenable is because it catches the exception and then tries to do this all over again. For example, with getStorage the deserialization in the then-call fails and returns to the catch exception from toThenable.

Changing this behaviour with Promise.resolve somehow changes the initialization of the stores. I had a test for a middleware where I explicitly checked that rehydration is called on init. With newImpl this test fails.

[swemail]

Unlike what I expected, #1517 issue was related with toThenable, as @meypod have thought.

#1518 may fix this issue too. Can @swemail or @MHC03 confirm?

I tried installing your branch but it does not seem to build the package then so I couldn't run it. How should I test it?

[dai-shi]

@swemail
https://ci.codesandbox.io/status/pmndrs/zustand/pr/1518
Find "Local Install Instruction" ☝️

[dai-shi]

@MHC03

I had a test for a middleware where I explicitly checked that rehydration is called on init. With newImpl this test fails.

I'm not sure if I follow. Do you agree #1518 fixes the regression? And, then it's still not ideal for both oldImpl and newImpl?

[swemail]

@swemail https://ci.codesandbox.io/status/pmndrs/zustand/pr/1518 Find "Local Install Instruction" ☝️

Thanks, still get the same issue in mocking with that fix and using storage. Works in the app with storage.

[dai-shi]

Oh, okay. Thanks for confirming. I think I misinterpreted @MHC03 's comment.

[MHC03]

No, it works for me, but I think toThenable is not ideal for both implementations, because it has some unexpected behaviour, at least for tests.

@swemail Your error message is a bit strange after looking at it again. Maybe we both had different problems. Could you try putting following code in the same file with the zustand mock at the top before create:

jest.mock('@react-native-async-storage/async-storage', () =>
  require('@react-native-async-storage/async-storage/jest/async-storage-mock')
);

[swemail]

No, it works for me, but I think toThenable is not ideal for both implementations, because it has some unexpected behaviour, at least for tests.

@swemail Your error message is a bit strange after looking at it again. Maybe we both had different problems. Could you try putting following code in the same file with the zustand mock at the top before create:

jest.mock('@react-native-async-storage/async-storage', () =>
  require('@react-native-async-storage/async-storage/jest/async-storage-mock')
);

With that addition it works :) Aaand with that addition the 4.2.0 works too so maybe the solution for this issue is only to add that in the documentation for how to mock with react-native async storage?

[dai-shi]

Would anyone open a PR to improve docs?

@MHC03 About toThenable, would you open a new discussion or a new issue with a reproduction with https://codesandbox.io/s/react-new ?

[MHC03]

I think this needs no additional docs.

@swemail Where and how have you created the mock directory and file for asyncstorage? When I use the directory method for mocking, it still works, maybe there is a typo somewhere.

@dai-shi I will try to make a reproducible example later.

[swemail]

I think this needs no additional docs.

@swemail Where and how have you created the mock directory and file for asyncstorage? When I use the directory method for mocking, it still works, maybe there is a typo somewhere.

@dai-shi I will try to make a reproducible example later.

Here is our directory setup for mocks

If I add this to our __mocks__/zustand.ts it works:

jest.mock('@react-native-async-storage/async-storage', () =>
  require('@react-native-async-storage/async-storage/jest/async-storage-mock'),
);

[MHC03]

I would export it exactly like the docs describe:
export default from '@react-native-async-storage/async-storage/jest/async-storage-mock';
Or you can also do it like this:

import AsyncStorageMock from '@react-native-async-storage/async-storage/jest/async-storage-mock';

export default AsyncStorageMock;

[swemail]

import AsyncStorageMock from '@react-native-async-storage/async-storage/jest/async-storage-mock';

export default AsyncStorageMock;

Now it works with the last one, doing it two steps! Sorry for causing confusion, it worked with our old way of mocking async-storage even if it was not exactly as prescribed so I looked in the wrong place 🙈

[MHC03]

Wanted to add, that I found following issue, which fits to the problem described in this issue: react-native-async-storage/async-storage#379

[dai-shi]

So, can close this issue, right?

[swemail]

So, can close this issue, right?

Yes! Thanks for the effort!

[wallacerenan]

Same with react-native-encrypted-storage