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.

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

Clarify online support #2927

Closed
TkDodo opened this issue Nov 11, 2021 · 23 comments
Closed

Clarify online support #2927

TkDodo opened this issue Nov 11, 2021 · 23 comments
Assignees
Labels
Milestone

Comments

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 11, 2021

We currently have multiple issues pertaining to online / offline support that are hard to fix without breaking something:

Proposed solution

Create a new query flag for offline queries (offline: boolean - name to be discussed). Since react-query is primarily an async state manager, it actually doesn't care about "being online". However, most people will use react-query for data fetching, so it makes sense to default this to false.

If you are using react-query to fetch from somewhere else, like AsyncStorage, where fetches should also happen when you are offline, you can set this flag to true. Also, if your api supports http caching, setting this to true will be beneficial because "fetching" from the browser-cache will also work when you're offline.

This would mean that we would, if offline is false (the default) and you have no internet:

  • pause retries when you have no internet. This would be non-breaking because we already do that.
  • Do not trigger refetches. Currently, we only do this for window focus refetches, but manual calls to refetch / invalidation don't adhere to this. If a query is explicitly refetched, it should probably be marked as stale instead so that it will be refetched on reconnect.
  • When a new query mounts, stay in whatever current state and don't trigger background refetches.
    • This means that if a query mounts for the first time, it will be in idle state - we need to properly communicate this that people need to handle that.
    • if a query is already in error state, we also wouldn't refetch, but stay in error state instead.

if offline is true, we would:

  • don't pause retries because you are offline - they would just continue. if you are not fetching via network, it would not be impacted, and if you have cached data in the browser cache, it would succeed on the first try anyways.
  • trigger refetches normally - same reasoning as above
  • when a new query mounts - also fetch normally (same reasoning as above)

If you say that your query is an offline query, but you are still fetching over the internet and have no browser cache, you will get a failed Promise after 3 retries and have to handle the error properly.


This new flag could potentially replace the refetchOnReconnect flag. If a query is an offline query, it wouldn't refetch on reconnect, and if it's an online query, it would. If we want to keep the flag, we could default it to the value of !offline. This is still to be discussed.

@TkDodo
Copy link
Collaborator Author

TkDodo commented Nov 12, 2021

one thing that is not addressed with the mentioned approach is the following scenario:

  • you have an online query
  • you start fetching, it takes some time
  • you go offline
  • the fetch will likely fail, triggering a retry
  • the retrier will pause retries because you are offline (and need to be online to continue)

--> your loading spinner will show until you're online again, even though you're not technically "fetching"

two solutions come to mind:

  1. remove the feature of pausing retries altogether. It means that once you have started fetching and go offline, it will continue to retry and will likely wind up in error state unless you go online again. I don't think that's ideal.

  2. cancel the query and revert it to its previous state. if its a background refetch, the query will go to success state and isFetching will be false, which is nice. If you don't have data yet, the query will go to idle state. I think this would be consistent with the behaviour when a query mounts and you're offline, so this would be my preferred solution.

What do you think @arnaudbzn ?

@TkDodo
Copy link
Collaborator Author

TkDodo commented Nov 12, 2021

Ah, another good way would be to maybe expose the internal isPaused flag. It should be true when the query is paused by the retrier. That way we could also potentially keep it in loading / fetching state, and consumers could check for isPaused. isFetching could also be exposed as false if isPaused is true, which would make background loading spinners disappear for paused queries 🤔

@arnaudbzn
Copy link
Contributor

arnaudbzn commented Nov 13, 2021

Ah, another good way would be to maybe expose the internal isPaused flag. It should be true when the query is paused by the retrier. That way we could also potentially keep it in loading / fetching state, and consumers could check for isPaused. isFetching could also be exposed as false if isPaused is true, which would make background loading spinners disappear for paused queries

I prefer this solution!
If isFetching is false, I can check isPaused if I need to display a "in pause" indicator!

A new pauseReason: offlinenotFocused can be handy to have a direct access to pause reason (it can be array of reasons).

onlineManager.setEventListener already provides a solution to customize what is an offline mode.
It means that the default concept of online/offline is associated to the network connectivity but it can be customized.
Same for the focus management.
So if a fetch the AsyncStorage, I have to use onlineManager.setEventListener to customize the online mode.
We could support a query based onlineManager.setEventListener for more granularity.

@TkDodo
Copy link
Collaborator Author

TkDodo commented Nov 13, 2021

I prefer this solution!
If isFetching is false, I can check isPaused if I need to display a "in pause" indicator!

spinning this idea a bit further:

type FetchingState = 'fetching' | 'paused' | 'idle'

we could of course provide isFetching and isPaused derived booleans from that internal state, similar to how query.state drives isSuccess, isError etc.

it would mean that if a new query mounts that doesn't have any data yet, if you are offline, instead of putting it in the idle state, we could just put it in:

status: 'loading',
fetchingStatus: 'paused'

sure, if you just display a loading spinner based on the status, you would see the spinner, but I think this is better than putting the query in idle state, which might not be handled at all by most people.

similar, if you have data already and mount a new query, the refetchOnMount would trigger but instead of actually fetching, it would put the query in:

status: 'success',
data: yourStaleData,
fetchingStatus: 'paused'

A new pauseReason: offline | notFocused can be handy to have a direct access to pause reason.

I think we could do that, too.

We could support a query based onlineManager.setEventListener for more granularity.

could you elaborate a bit on how that could look like?


I'm still not totally happy with the fact that you have to decide between online / offline queries. For example, you can't know if a query will yield data from the browser cache or not. If it does, mounting it, fetching + getting data from the browser cache would work, but with the initial proposal, you would have to specify it as an offline query because it otherwise wouldn't start fetching. But if you do that and there is no browser cache data available for whatever reason, retries wouldn't pause :/
So essentially, in this situation, trying the fetch once and then pausing the retries would be nice, which is essentially what we are doing today. However, if you for example turn off retries and are offline, we fetch and would give you the error immediately, which would spark the question of why did we even try to fetch at all...

@arnaudbzn
Copy link
Contributor

arnaudbzn commented Nov 13, 2021

We could support a query based onlineManager.setEventListener for more granularity.
could you elaborate a bit on how that could look like?

I can have one query for the AsyncStorage and another one for an API. The same concept of offline cannot apply for both.
setEventListener could accept a new queryKey argument. So my AsyncStorage based query can be always online.

@TkDodo
Copy link
Collaborator Author

TkDodo commented Nov 13, 2021

So my AsyncStorage based query can be always online.

I think this is exactly the case I wanted to solve with the offline query option. It could be set on a per-query level to define which queries need to be online to work and which don't.

The onlineManager is a global singleton, which doesn't have any correlation to queries. You either are online, or you're not (I think...)

@arnaudbzn
Copy link
Contributor

arnaudbzn commented Nov 13, 2021

I think this is exactly the case I wanted to solve with the offline query option. It could be set on a per-query level to define which queries need to be online to work and which don't.

The naming could be more precise, instead of offline:

  • offlineModeSupport: true
    or
  • mode: online (default) | offline | all

@TkDodo
Copy link
Collaborator Author

TkDodo commented Nov 13, 2021

yes, definitely agree. I like mode: 'online' | 'offline' - but what would 'all' be 😅 ?

@arnaudbzn
Copy link
Contributor

arnaudbzn commented Nov 13, 2021

yes, definitely agree. I like mode: 'online' | 'offline' - but what would 'all' be 😅 ?

all it's weird indeed.
I was just thinking about 3 possible modes

  • online
  • offline only
  • online and offline (all)

offline only mode means that the query isPaused: true when the connectivity is online, but I have no use case in mind for this mode..

mode: 'online' | 'offline' is straightforward!

@Liam-Tait
Copy link
Contributor

What about requiresInternetConnection: boolean?

I imagine this would be able to be set as a config value in QueryClient and it would be rare to override that value on a per query basis. This means the verboseness of the name is less of an issue

@TkDodo
Copy link
Collaborator Author

TkDodo commented Nov 17, 2021

networkMode: 'online' | 'offline'
networkRetry: boolean | () => boolean // defaults to networkMode === 'online'

@TkDodo TkDodo self-assigned this Nov 20, 2021
@arnaudbzn
Copy link
Contributor

networkMode: 'online' | 'offline'

Just for clarification:
Does it mean that a query cannot have an offline-only network mode?

@TkDodo
Copy link
Collaborator Author

TkDodo commented Nov 20, 2021

@arnaudbzn it can. I have since refined the api again, and almost have a PR ready. My current solution is:

networkMode: 'online' | 'always' | 'offlineFirst'

I'll also write docs on it, but general, online (the default) means:

  • queries will not start or refetch unless you have internet - they will go to isPaused in addition to its normal state (can be loading if it mounts for the first time, or success we already have background data)
  • if a query fetches (because you are online), but then loose connection, the retries will also pause
  • paused queries will continue once you reconnect, and refetchOnReconnect defaults to true here as well.

always means:

  • react-query makes no assumption and always tries to fetch, no matter the online status. This is what you want when you fetch from localhost or AsyncStorage or just want to return Promise.resolve(5) because network is not involved at all
  • retries are never paused and just continue to fire. if you use this mode and still fetch over the network while being offline, you'll go to error state eventually with a Network Error or whatever it is that fetch/axios give you in this case.
  • refetchOnReconnect is false here (per default, you can still override) because its not a good indicator to refetch if you reconnect

offlineFirst is the middle ground and means:

  • react-query will try to fire off the first fetch, but pause retries if you are offline. This is very handy if you have a serviceWorker that intercepts a request for caching like an offline-first PWA, or if you use http caching headers with max-age. Service workers usually intercept fetch requests, so they need to happen even if you are offline, and browser caches will also return the data if something is in the cache when you fetch even if you are offline. However, if there is a cache miss, the network request will go out and fail, in which case we want to behave like an online query - so pausing retries.
  • same for refetches: try the first but pause retries if offline
  • refetchOnReconnect is also true here

hope that makes sense and covers all cases. I've also combined isFetching and isPaused to an internal fetchStatus: paused | fetching | idle and the booleans will be derived from it, but the whole state is exposed, too. So useIsFetching will for example not return paused queries - only really fetching ones, as the use-case here is to display background loading spinners, which you likely don't want to see when a query is paused because you're offline :)

@arnaudbzn
Copy link
Contributor

networkMode: 'online' | 'always' | 'offlineFirst'

Great!

@salembaira
Copy link

@TkDodo really well thought! I cant wait to try this as I'm a big fan of offline-first apps.

@TkDodo
Copy link
Collaborator Author

TkDodo commented Nov 28, 2021

@baira I think I’m done with the PR and would appreciate feedback, checkout the codesandbox preview build if you want:

#3006

note that you need to change the query key to be an array as this is required in v4 but we haven’t adapted the examples yet 😅

@TkDodo TkDodo closed this as completed Dec 1, 2021
@goatbarn
Copy link

Hello there, I'm very curious as to this feature...so much so that I am testing it out in my test/internal work app.

Is the goal of the persistentQueryClient to auto-dehydrate/hydrate the data on connectivity change?

Or is this something that would need to be setup on a per basis use case?

Currently I'm trying the alpha release for this purpose soley, as I have the condition that a user may want to submit (post/mutate) data whilst their offline. If they get internet back while their still in app, great, I can just merge errors on resulting onSuccess callback (that will be missing the data)...but what about if they don't and they restart the app with internet, this would be where I find any existing "persisted" cache, and hydrate it before first fetch.

I apologize for my language, I'm a self taught everything-related-to-this guy. Thanks!

@TkDodo
Copy link
Collaborator Author

TkDodo commented Jan 14, 2022

Hello there, I'm very curious as to this feature...so much so that I am testing it out in my test/internal work app.

awesome, thank you for your feedback 🙌

Is the goal of the persistentQueryClient to auto-dehydrate/hydrate the data on connectivity change?

no, the persistQueryClient is not tied to connectivity change at all. If you call persistQueryClient, you create a subscription to the query cache, which will write every change directly to localStorage (with a throttling). Additionally, it will load the data from the localStorage once into the queryCache.

this can be used at the start of the app to keep the localStorage in sync with your app, and to initially hydrate it.

as of today, on v4, we have some more options:

this should give you even more control over when to read from / write to an external storage.

Currently I'm trying the alpha release for this purpose soley, as I have the condition that a user may want to submit (post/mutate) data whilst their offline. If they get internet back while their still in app, great, I can just merge errors on resulting onSuccess callback (that will be missing the data)...but what about if they don't and they restart the app with internet, this would be where I find any existing "persisted" cache, and hydrate it before first fetch.

for this case, I think you need to resumePausedMutations when the app starts, as described here:
https://react-query.tanstack.com/guides/mutations#persist-mutations

@goatbarn
Copy link

Awesome, yes I'm doing that currently. I suppose I'm wishfully thinking I can extend this new persistence feature to cover a sudo "offline sync" function.

I see now it's complicated enough that services cover this space, like Realm.

My user base isn't large enough to warrant that yet, though. I think if I were to utilize the newest, v4 persist features you mentioned along with possibly extending the class to listen to the onlineManager, and when offline switch my api url to that of a local database that I then update and cache until the internet is back on. Then I'd have to implement a sync feature with lastUpdated-at type fields to try and merge.

I think it's only doable for me now this way as I have control over the front/back and have less than 50 users.

If my gibberish anove makes any sense, please feel free to offer advice on my idea.

Anyway. Great job on the library. It's in my template I start with on every new build!

@morismoze
Copy link

morismoze commented Mar 19, 2022

@arnaudbzn it can. I have since refined the api again, and almost have a PR ready. My current solution is:

networkMode: 'online' | 'always' | 'offlineFirst'

I'll also write docs on it, but general, online (the default) means:

  • queries will not start or refetch unless you have internet - they will go to isPaused in addition to its normal state (can be loading if it mounts for the first time, or success we already have background data)
  • if a query fetches (because you are online), but then loose connection, the retries will also pause
  • paused queries will continue once you reconnect, and refetchOnReconnect defaults to true here as well.

always means:

  • react-query makes no assumption and always tries to fetch, no matter the online status. This is what you want when you fetch from localhost or AsyncStorage or just want to return Promise.resolve(5) because network is not involved at all
  • retries are never paused and just continue to fire. if you use this mode and still fetch over the network while being offline, you'll go to error state eventually with a Network Error or whatever it is that fetch/axios give you in this case.
  • refetchOnReconnect is false here (per default, you can still override) because its not a good indicator to refetch if you reconnect

offlineFirst is the middle ground and means:

  • react-query will try to fire off the first fetch, but pause retries if you are offline. This is very handy if you have a serviceWorker that intercepts a request for caching like an offline-first PWA, or if you use http caching headers with max-age. Service workers usually intercept fetch requests, so they need to happen even if you are offline, and browser caches will also return the data if something is in the cache when you fetch even if you are offline. However, if there is a cache miss, the network request will go out and fail, in which case we want to behave like an online query - so pausing retries.
  • same for refetches: try the first but pause retries if offline
  • refetchOnReconnect is also true here

hope that makes sense and covers all cases. I've also combined isFetching and isPaused to an internal fetchStatus: paused | fetching | idle and the booleans will be derived from it, but the whole state is exposed, too. So useIsFetching will for example not return paused queries - only really fetching ones, as the use-case here is to display background loading spinners, which you likely don't want to see when a query is paused because you're offline :)

Hi!

Version: 4.0.0-alpha.20
Platform: React Native

Just installed the new version in purpose of using 'networkMode': 'online' so refetching doesn't happen when Internet connection is lost (also it is important to note that the query didn't start nor is in loading/fetching state when connection is lost - it is idled - and it is also not mounting for the first time).

The problem is that for some reason as soon as I turn off connection on real device, isFetching flag turns to 'true' for like 1-2 seconds and then goes to 'false'. Is this the intended behaviour? By "queries will not start or refetch unless you have internet" I would say it is not.

Despite that, how to tackle the fact that on the first mount query starts loading even though there is no Internet connection?

Thanks!

@TkDodo
Copy link
Collaborator Author

TkDodo commented Mar 19, 2022

@morismoze thanks for trying out v4!

The problem is that for some reason as soon as I turn off connection on real device, isFetching flag turns to 'true' for like 1-2 seconds and then goes to 'false'. Is this the intended behaviour? By "queries will not start or refetch unless you have internet" I would say it is not.

No, it should not. Can you reproduce this in a codesandbox, or in an expo snack?

Despite that, how to tackle the fact that on the first mount query starts loading even though there is no Internet connection?

When a query mounts for the first time and you have no internet, it should be in status: 'loading' with fetchStatus: 'paused'. It's loading because it doesn't have data yet, and paused because there is no internet connection.

@morismoze
Copy link

When a query mounts for the first time and you have no internet, it should be in status: 'loading' with fetchStatus: 'paused'. It's loading because it doesn't have data yet, and paused because there is no internet connection.

Managed to work it out with isPaused flag, thank You!

No, it should not. Can you reproduce this in a codesandbox, or in an expo snack?

Tried with both but can't reproduce wanted behaviour. Also forgot to mention that I'm using https://react-query.tanstack.com/react-native#online-status-management and https://react-query.tanstack.com/react-native#refetch-on-app-focus, but I think those two shouldn't have any influence on that particular issue?

I'll try to explain the case once more (since it is the only way): I'm on Home screen, everything is fetched, so there is no fetching going on - everything is idled. Then I shut off mobile data and isFetching turns to true for 1-2 seconds and then falls back to false

@TkDodo
Copy link
Collaborator Author

TkDodo commented Mar 24, 2022

Managed to work it out with isPaused flag, thank You!

👍

I'll try to explain the case once more (since it is the only way): I'm on Home screen, everything is fetched, so there is no fetching going on - everything is idled. Then I shut off mobile data and isFetching turns to true for 1-2 seconds and then falls back to false

I cannot imagine isFetching going to true just because you turn off the internet. Maybe you can try to turn off the extra online status management / refetch on app focus one by one to see if that influences anything?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants