-
Notifications
You must be signed in to change notification settings - Fork 462
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
Revisiting Hooks implementation #32
Comments
For now, I do it like this:
The value of TextInput is now persist. |
Maybe using a value to keep track of the status (updated or not) of the value would help? useEffect(() => {
getStorageValue();
}, [updated]); |
const useAsyncStorage = (key, defaultValue) => {
const [storageValue, updateStorageValue] = useState(defaultValue);
const [updated, setUpdated] = useState(false);
async function getStorageValue() {
let value = defaultValue;
try {
value = JSON.parse(await AsyncStorage.getItem(key)) || defaultValue;
} catch (e) {
} finally {
updateStorageValue(value);
setUpdated(true);
}
}
async function updateStorage(newValue) {
try {
if (newValue === null) {
await AsyncStorage.removeItem(key);
} else {
const value = JSON.stringify(newValue);
await AsyncStorage.setItem(key, value);
}
} catch (e) {
} finally {
setUpdated(false);
getStorageValue();
}
}
useEffect(() => {
getStorageValue();
}, [updated]);
return [storageValue, updateStorage];
}; |
It still returns the default value in the first time. Can we get the actual stored data in the first time? |
We're going to revisit it in v2. |
I iterated on the previous version @edwinvrgs proposed and came up with this (in Typescript): import AsyncStorage from '@react-native-community/async-storage'
import { useEffect, useState } from 'react'
const useAsyncStorage = <T>(key: string, defaultValue: T): [T, (newValue: T) => void, boolean] => {
const [state, setState] = useState({
hydrated: false,
storageValue: defaultValue
})
const { hydrated, storageValue } = state
async function pullFromStorage() {
const fromStorage = await AsyncStorage.getItem(key)
let value = defaultValue
if (fromStorage) {
value = JSON.parse(fromStorage)
}
setState({ hydrated: true, storageValue: value });
}
async function updateStorage(newValue: T) {
setState({ hydrated: true, storageValue: newValue })
const stringifiedValue = JSON.stringify(newValue);
await AsyncStorage.setItem(key, stringifiedValue);
}
useEffect(() => {
pullFromStorage();
}, []);
return [storageValue, updateStorage, hydrated];
};
export default useAsyncStorage A couple notable tweaks:
Note: I took out the error-handling for now, because I don't need it for my use-case |
Man @dkniffin, great approach. I came up with a similar solution in my current project but nothing as polish as this proposal of you. The only thing that I can say is that maybe we can use |
@edwinvrgs Yep, agreed. I think that makes sense. I've been working with this a bit more and tweaking it as a I go. The two notable changes I've made so far are:
|
My team has been using this solution I wrote on an app we're working on. We've been using it for a while now, and there's one gotcha we've just run into: We fixed that by having the form state stored separately in memory and only write to it at specific times (when the user clicks a "Next" button). I could see that solution being incorporated into the snippet I wrote above (which ideally I'm hoping gets integrated into this library at some point), so that this On this topic, @krizzu is this hook revisit still in the works for v2? When should we expect that? |
Reopening for further discussions to improve hooks for v1. @dkniffin I know it's been a while, but your solution looks pretty solid. What about |
@krizzu I'm not sure what you mean by As for this solution, you are welcome to use it. However, I am no longer working on the project that I implemented that in, so I can no longer attest to whether it's working or not, unfortunately, and I'm not actually working on any RN projects at the moment. Sorry. |
Can't
On the other hand, maybe such values should be loaded outside the component that needs them in the first place? Then the async nature of the refresh when the value is actually retrieved is not that obvious? |
@pke No, since the This means, that there will always be at least one unavoidable re-render. |
Thanks @dkniffin Here is about the same implementation using useCallback import AsyncStorage from '@react-native-async-storage/async-storage'
import * as React from 'react'
import log from '../utils/log'
const useStoredState = <T>(
key: string,
defaultValue: T,
): [T, (newValue: T) => void, boolean] => {
const [state, setState] = React.useState({
hydrated: false,
storageValue: defaultValue,
})
const {hydrated, storageValue} = state
React.useEffect(() => {
const pullFromStorage = async () => {
let value = defaultValue
try {
const fromStorage = await AsyncStorage.getItem(key)
if (fromStorage) {
value = JSON.parse(fromStorage)
}
} catch (e) {
log('Could not read from storage for key: ', key, e)
}
return value
}
pullFromStorage().then((value) => {
setState({hydrated: true, storageValue: value})
})
// We don't want to update when the defaultValue changes
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [key])
const updateStorage = React.useCallback(
async (newValue: T) => {
setState({hydrated: true, storageValue: newValue})
const stringifiedValue = JSON.stringify(newValue)
await AsyncStorage.setItem(key, stringifiedValue)
},
[key],
)
return [storageValue, updateStorage, hydrated]
}
export default useStoredState |
This issue has been marked as stale due to inactivity. Please respond or otherwise resolve the issue within 7 days or it will be closed. |
Here is my version with error state. import AsyncStorage from '@react-native-async-storage/async-storage';
import { useEffect, useState, useCallback } from 'react';
export default function useAsyncStorage<T>(key: string, defaultValue: T): {
value?: T,
setValue: (newValue: T) => Promise<void>,
removeValue: () => Promise<void>,
loading?: boolean,
error?: Error,
} {
const [error, setError] = useState<Error | undefined>();
const [loading, setLoading] = useState<boolean>(true);
const [value, setValue] = useState<T | undefined>();
async function getValueFromAsyncStorage(): Promise<void> {
try {
setLoading(true);
setError(undefined);
setValue(defaultValue);
const value = await AsyncStorage.getItem(key);
if (value !== null) {
setValue(JSON.parse(value));
}
} catch (error: any) {
setError(error);
} finally {
setLoading(false);
}
}
useEffect(() => {
getValueFromAsyncStorage();
}, [key])
const handleSetValue = useCallback(async (newValue: T) => {
const stringifiedValue = JSON.stringify(newValue);
await AsyncStorage.setItem(key, stringifiedValue);
setError(undefined);
setValue(newValue);
setLoading(false);
}, [key]);
const handleRemoveValue = useCallback(async () => {
await AsyncStorage.removeItem(key);
setError(undefined);
setValue(undefined);
setLoading(false);
}, [key]);
return {
value,
setValue: handleSetValue,
removeValue: handleRemoveValue,
loading,
error,
};
} |
I have created #760 to fix the current hook and also introduce a proper hook, which supports basically what everbody expected the hook to do in the first place: exposing a current value and an update function. @seeden Thansk for your version! Your version could be improved by combining the states into one and with all the various versions of hooks I have seen suggested here, I always wondered what the client code is supposed to do in the error case. Face the user with it and offer a "retry" action to save the setting again? What if not one but multiple items can't be saved? Also my version does not contain loading state, cause I think one should not be seeing storage values to be loaded, it should be fast. Please join the discussion of the PR and maybe we can include loading and error states. Or maybe we use getter and setter for the value and you can just write
Of course that would not allow one to use the setting to be directly assigned to an |
this implementation should do the job for updating value across all hooked components.
and these are some use examples;
or also if we want to parse the returned data we can do;
|
Alright, I'll throw my hat in the ring here too. This hook updates the component onFocus, which is useful if components on seperate screens are updating the same storage value and the user swipes back (in which case a new render isn't triggered) import { useCallback, useEffect, useState } from "react";
import AsyncStorage from "@react-native-async-storage/async-storage";
import { useFocusEffect } from "@react-navigation/native";
export function useAsyncStorage<T>(
key: string,
defaultValue: T
): [T, (nextValue: T) => Promise<void>] {
const [value, setValue] = useState(defaultValue);
const [updated, setUpdated] = useState(false);
useFocusEffect(
useCallback(() => {
getStorageValue();
}, [])
);
useEffect(() => {
if (!updated) {
getStorageValue();
}
}, [updated]);
async function getStorageValue() {
let nextValue = defaultValue;
const fromStorage = await AsyncStorage.getItem(key);
if (fromStorage) {
nextValue = JSON.parse(fromStorage);
}
setValue(nextValue);
setUpdated(true);
}
async function setStorageValue(nextValue: T) {
await AsyncStorage.setItem(key, JSON.stringify(nextValue));
setUpdated(false);
}
return [value, setStorageValue];
} |
I would put the state in a context provider for this, so I get a global state that I can use everywhere in my app. Thinking about possible side effects of writing and reading from storage is horrible. Also what happens if you need to use the state two times in the same screen? |
Thanks for the feedback. Using Context can be good, except that the entire app rerenders which can be undesirable. but you're correct - two components on the same screen pointing to the same key in asyncStorage will not update each other. |
I can't think of a solution to the "always returns default value during 1st render" problem without implementing a "sync access" to storage. (similar to 2 active hooks that read from the same key will also not know if the other hook changes the value in storage. we can maybe implement
this store can also notify all active hooks about a change made to a key from anywhere in the app. |
If I am using the |
You want to:
Discuss required changes to current 'hook-ish' implementation for Hooks support.
Details:
As you can see here, we've got a nice wrapper on top
AsyncStorage
, to mimic hooks usage.This discussion here is to come up with better and valid implementation, that leverages hooks API.
Example implementation could look like this:
The problem in here is that, once we got a value inside AsyncStorage, we'll be getting two rerenders one component mounting - one returning the default value, second the actual stored data.
I'm more than happy to discuss this, so please let me know what you think.
The text was updated successfully, but these errors were encountered: