Skip to content

Conversation

@Aung-Myint-Thein
Copy link
Contributor

@Aung-Myint-Thein Aung-Myint-Thein commented Jun 9, 2021

Implementation

I looked at the createLocalStoragePersistor and just changed a few lines and necessary imports. Please feel free to suggest if there is anything I should change or improve.

How to use

import {asyncStoragePersistor} from './utility/createAsyncStoragePersistor';

const queryClient = new QueryClient({
  defaultOptions: {
    queries: {
      cacheTime: 1000 * 60 * 60 * 48, // 48 hours
      staleTime: 1000 * 60 * 60 * 24, // 24 hours
    },
  },
});

const storagePersistor = asyncStoragePersistor({
  asyncStorageKey: 'AsyncStorageReactQuery', // asyncStorageKey vs localStorageKey
  throttleTime: 1000,
});

persistQueryClient({
  queryClient,
  persistor: storagePersistor,
  maxAge: 1000 * 60 * 60 * 24, // 24 hours in milliseconds
});

How to test

Use some queries to get the data. Restart the React Native app. We should see the UIs with previously called queries immediately without needing to call the APIs.

@vercel
Copy link

vercel bot commented Jun 9, 2021

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/tannerlinsley/react-query/A3EyaNQsLHX5dPi8hA8NDXxqSaCX
✅ Preview: https://react-query-git-fork-aung-myint-thein-asyn-4979ec-tannerlinsley.vercel.app

@Aung-Myint-Thein Aung-Myint-Thein changed the title added a new file for asyncStoragePersistor asyncStoragePersistor for React Native Jun 9, 2021
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 9, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 279faaa:

Sandbox Source
tannerlinsley/react-query: basic Configuration
tannerlinsley/react-query: basic-typescript Configuration

TkDodo
TkDodo previously requested changes Jun 9, 2021
Copy link
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

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

looks good, but could you please streamline it with the other plugins, meaning:

  • typescript
  • add to tsconfig.types.json
  • docs for the plugins
  • package.json
  • add it to the rollup config

basically, if you search for createLocalStoragePersistor-experimental everywhere, you'll see what you'll need to add :)

@Aung-Myint-Thein
Copy link
Contributor Author

looks good, but could you please streamline it with the other plugins, meaning:

  • typescript
  • add to tsconfig.types.json
  • docs for the plugins
  • package.json
  • add it to the rollup config

basically, if you search for createLocalStoragePersistor-experimental everywhere, you'll see what you'll need to add :)

Added docs for plugins. Sorry, I am not a type script person and.. not very familiar with all the rollup config... :(

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 9, 2021

no worries. I think I can check out the branch and add some commits I guess?

@Aung-Myint-Thein
Copy link
Contributor Author

Please feel free to add more commits. I can verify that the code is working coz I am running the codes now and tested a few times by restarting the app and can confirm querycache data is there.

@arnaudbzn
Copy link
Contributor

@TkDodo @Aung-Myint-Thein We are going to test this PR as well in the coming days. We'll keep you updated if we have any issues or comments.

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 11, 2021

can you please update your fork and branch with the latest release, as we've now made some changes to the webstorageplugin and I would like to avoid conflicts


export const asyncStoragePersistor = ({asyncStorageKey, throttleTime}) => {
return {
persistClient: throttle(function (persistedClient) {
Copy link

Choose a reason for hiding this comment

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

I'm not sure whether we should use throttle here because setItem is async so probably we want to return this promise as well :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 14, 2021

build fails because:

Cannot find module '@react-native-community/async-storage' or its corresponding type declarations.

How are we going to go about this please? I'd rather not add this as a peerDependency. Maybe it's better to change the interface to pass the actual AsyncStorage as a parameter to asyncStoragePersistor ? We've done something similar to the WebStoragePersistor...

@TMaszko
Copy link

TMaszko commented Jun 14, 2021

@TkDodo What about async setItem ?

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 14, 2021

@TkDodo What about async setItem ?

Yes, we'd need something as a parameter that has an async getItem and an async setItem method :)

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 14, 2021

we'd need something as a parameter that has an async getItem and an async setItem method :)

tried that here: 0612612

@TMaszko
Copy link

TMaszko commented Jun 14, 2021

k, but we cannot just not throttle, can we? we can make it an async function to have it always return a Promise?

I think so :D

@TMaszko
Copy link

TMaszko commented Jun 15, 2021

@TkDodo we could use smth like p-throttle https://github.com/sindresorhus/p-throttle wdyt?

Persisor -> Persistor
@TkDodo
Copy link
Collaborator

TkDodo commented Jun 15, 2021

@TkDodo we could use smth like p-throttle https://github.com/sindresorhus/p-throttle wdyt?

what would be the drawback of just using the normal throttle that doesn't return anything?

@TMaszko
Copy link

TMaszko commented Jun 15, 2021

@TkDodo Probably, there we could run into some race conditions. I mean one operation could be faster than the other one but the order is not maintained on JS side, but on the otherhand saveClient is called by the subscribe method and it's not "awaited" at all...

@TMaszko
Copy link

TMaszko commented Jun 16, 2021

@tannerlinsley
Copy link
Collaborator

This is all looking really good. I think the changes to make things more flexible is the right move. Inverting control out of the API to the user is always a good decision, same way we did it with the other web storage api's.

Keep cruisin'!

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 17, 2021

@TMaszko thanks, I used your asyncThrottle implementation instead of throttle here: 73a20c4

@TkDodo TkDodo dismissed their stale review June 17, 2021 07:06

dismissing because I implemented lots of it myself :)

@dan-lee
Copy link

dan-lee commented Jun 17, 2021

I created a third-party module for this exact purpose a while ago: https://github.com/dan-lee/react-query-native-persist

It can handle multiple storage backends (with the help of adapters). Maybe this can serve as inspiration regarding @tannerlinsley's point in his last comment. I'd also be willing to contribute if wanted.

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 18, 2021

@TMaszko @Aung-Myint-Thein the PR would be good to go from my side, but I haven't really tested it. If someone can make a quick test with AsyncStorage, and maybe as well with localForage to see if it's compatible with the api, that would be much appreciated

@TMaszko
Copy link

TMaszko commented Jun 18, 2021

@TkDodo I've already tested AsyncStorage persistor in the project but in the form of custom persistor (gist code) and It was working correctly.

Co-authored-by: Tomasz Krzyżowski <t.krzyzowski96@gmail.com>
@idanavr
Copy link

idanavr commented Jun 27, 2021

When will it be merged?

@TkDodo TkDodo merged commit 7a628d6 into TanStack:master Jun 28, 2021
@tannerlinsley
Copy link
Collaborator

🎉 This PR is included in version 3.18.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants