Add support for async storages (e.g. AsyncStorage for React Native)

[jgornick]

Taking a quick look at the code, it doesn't seem like there's support for awaiting on getItem, setItem or removeItem from the storage instance.

Would a change like this be as simple as converting the methods to support async/await?

Do you see any other challenges with making the change?

[soyguijarro]

Hey @jgornick, thanks for the suggestion. Off the top of my head, yes, I think that would be all.

[jgornick]

Expect a PR soon 😄

[jgornick]

Started to work on the change here: https://github.com/jgornick/react-storage-hooks/blob/14-add-async-storage-support/src/common.ts

Any feedback or help would be greatly appreciated 😄

[soyguijarro]

Started to work on the change here: https://github.com/jgornick/react-storage-hooks/blob/14-add-async-storage-support/src/common.ts

Any feedback or help would be greatly appreciated smile

Looking good at a quick glance! Feel free to open a PR if you want and I'll take a closer look, but please make sure to add some test cases to cover the new async behavior. Thanks!

[jgornick]

@soyguijarro Looking at this a bit more, this is a bit more difficult than I originally thought.

A couple of concerns:

• Since our storage methods are now async, this means that the external API for methods like useStorageState and useStorageReducer will most likely change. I'm not a hooks expert, but based on other async related hooks, the result includes a "loading" flag to know when the state is available to be read and updated.
• Again, now that everything is async, we would have to wrap most, if not all, of the hooks publicly and privately with useEffect. Also, similar to the first concern, we would have to update the APIs to support a "loading" flag of sorts.

Again, I must admit, I'm not a hooks expert. However, the more I dig into this, the more it seems like a breaking change more than anything. If I'm not far off on this, as the owner of the library, are you OK with breaking API changes? Do we split out async support into their own hooks?

Thoughts?

[jgornick]

Just as an update, I did start to look at ways to implement useEffect as means to handle async storage methods: https://github.com/jgornick/react-storage-hooks/blob/14-add-async-storage-support/src/state.ts#L21-L37

The change to the behavior of the hook then is that the state returned in the tuple will be null to start, then after the useEffect is called and the promise for reading the item from storage, the state will be updated.

I also started to modify the tests for the hook as well: https://github.com/jgornick/react-storage-hooks/blob/14-add-async-storage-support/src/tests/state.test.ts#L16-L27. They use the waitForNextUpdate method returned from the renderHook test utility.

Any feedback would be greatly appreciated!

[soyguijarro]

You're totally right @jgornick, this would be a breaking change, but I'm fine with that. I think having async support would solve this SSR issue as well, so it's probably worth doing.

Having sync and async versions of the hooks might look like it simplifies the sync use case, but it introduces complexity for users anyway: you now need to choose between the two hook flavors which have slightly different APIs, switch between them if you change your storage... Not worth it IMHO. I'd go with converting everything to async. For consumers using sync storages like localStorage, if your application doesn't break with the initial undefined you might not even care about the loading state since it'll be super short.

[jgornick]

@soyguijarro I'm not quite sure I know exactly what I'm doing, but I've made a commit that shows the updated interface for the state hook. I've also started to make some changes to the reducer hook, but not finished.

Any chance you can have a look so far with what's there and if I should continue with updating tests?

Thanks!

[jgornick]

Let's move the conversation into #20 .