Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Cache persistence based on AsyncStorage #265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Cache persistence based on AsyncStorage #265

wants to merge 1 commit into from

Conversation

JesuHrz
Copy link

@JesuHrz JesuHrz commented Mar 8, 2022 •
edited
Loading

This PR adds a new approach for cache persistence based on AsyncStorage in React Native

@JesuHrz JesuHrz requested review from shuding and huozhi as code owners March 8, 2022 19:13
@vercel
Copy link

vercel bot commented Mar 8, 2022 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/swr-site/CRq5E9B7GBeTGUUzB5hEn9UJi86q
✅ Preview: https://swr-site-git-fork-jesuhrz-jesuhrz-persist-cache-based-on-bf4586.vercel.sh

@JesuHrz JesuHrz force-pushed the jesuhrz/persist-cache-based-on-async-storage branch from 5aab9c8 to 4c214c4 Compare March 8, 2022 19:14
@JesuHrz JesuHrz force-pushed the jesuhrz/persist-cache-based-on-async-storage branch from 4c214c4 to fe14c0f Compare March 8, 2022 19:18
@JesuHrz JesuHrz force-pushed the jesuhrz/persist-cache-based-on-async-storage branch from fe14c0f to b025124 Compare March 8, 2022 19:19
@JesuHrz JesuHrz force-pushed the jesuhrz/persist-cache-based-on-async-storage branch from b025124 to 714e497 Compare March 9, 2022 02:13
@JesuHrz JesuHrz force-pushed the jesuhrz/persist-cache-based-on-async-storage branch from 714e497 to 98e6571 Compare March 9, 2022 02:15
huozhi
huozhi reviewed Apr 13, 2022
View reviewed changes
pages/docs/advanced/react-native.en-US.mdx Outdated
const useAsyncStorage = () => {
const [cache, setCache] = useState(null)

useLayoutEffect(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example looks good, just 1 question: is there any reason use useLayoutEffect here instead of useEffect?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, you're right
Thanks for the feedback, I'm gonna update this one @huozhi

shuding
shuding reviewed May 8, 2022
View reviewed changes
Copy link
Member

@shuding shuding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work. Cache initialization should happen inside the provider function other than outside, because SWR creates the cache instance on initial render.

Also, a more reliable way to write back to the persistent storage is to register an onbeforeunload listener (or similar method in RN), instead of doing it on unmount.

@JesuHrz
Copy link
Author

JesuHrz commented May 9, 2022 •
edited
Loading

I do not understand why you say it will not work, if the SWR instance is only generated once the cache is initialized, as long as it is not initialized SWRConfig will never be rendered.

Also remember that AsyncStorage is always returns a promise, and the provider function does not allow it to be a promise

It would be great if you have the opportunity to test this workaround, because on my side it works without any problem
@shuding

@JesuHrz JesuHrz force-pushed the jesuhrz/persist-cache-based-on-async-storage branch from 98e6571 to 553ab12 Compare May 9, 2022 22:04
@vercel
Copy link

vercel bot commented May 9, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
swr-site ✅ Ready (Inspect) Visit Preview May 9, 2022 at 10:06PM (UTC)

@JesuHrz
Copy link
Author

JesuHrz commented May 9, 2022

@JesuHrz JesuHrz closed this May 9, 2022
@JesuHrz JesuHrz reopened this May 9, 2022
@JesuHrz JesuHrz requested review from huozhi and shuding May 9, 2022 22:14
@JesuHrz
Copy link
Author

JesuHrz commented Aug 9, 2022

Hey guys @shuding @huozhi, do you have any update about this MR?
If you guys have any feedback, please let me know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@huozhi huozhi Awaiting requested review from huozhi huozhi is a code owner

@shuding shuding Awaiting requested review from shuding shuding is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JesuHrz @shuding @huozhi