feat(background-fetching): Introduce isFetching in QueryState to handle refetch

[trollepierre]

Context:

We want to persist data from Cozy-Client, inside a storage, in order to reuse them for next loading of the app (without send any request to Pouch / Stack). Redux-persist helps us to persist redux store.

However, when the data got rehydrated, the requests have already a fetchStatus "loaded", so they are not updated by Cozy-Client, because the data already existed.

That's why we used fetchPolicy, to refresh data older than a certain amount of hours / days.
However there are some drawbacks:

• a delay value is totally arbitrary, so not reliable
• when fetchPolicy is not respected, then the query is reexecuted, passing the fetchStatus to loading again, that make application displaying a Spinner, which is bad for User Experience.

This PR follows the same concept of background fetching from React Query (https://react-query.tanstack.com/guides/background-fetching-indicators & https://react-query.tanstack.com/guides/window-focus-refetching)

Implementation

• a query has a loaded fetchStatus
• this query is restarted (by another component or a rehydratation)
• then the query keeps the loaded fetchStatus, the applicaations keep displaying the data, while the request got executing in background. A isFetching: true attribute is added inside the QueryState
• When CozyClient finally load the data to the store, the application refreshes instantly with fresh data

This behavior should only concern React part of Cozy Client (useQuery, ObservableQuery). The client.query() should not implement this, because inside Node usage, we will have issues.

[Crash--]

Few remarks:

• Maybe we should add a per query attribute to enable background Fetching. And maybe if this attribute is here, we should override the fetchPolicy?
• Maybe we should add a global attribute to enable background fetching. I think we already have something to declare global error handler.

[trollepierre]

Few remarks:
Maybe we should add a per query attribute to enable background Fetching. And maybe if this attribute is here, we should override the fetchPolicy?
Maybe we should add a global attribute to enable background fetching. I think we already have something to declare global error handler.

Clear. I think it would be better for developer experience to control the backgroundFetching of each query. I would better go for the "per query attribute" (inside QueryOptions)

• do you think we should support both options (the global attribute AND the "per query" attribute)?

[Crash--]

do you think we should support both options (the global attribute AND the "per query" attribute)?

I think we should follow what we have done for the onError callback: We can add it globally or having one per query (c5f65d3)

[Crash--]

I'm really not sure about that. fetch policy is useful for caching and to avoid fetch "spam" when rendering several components.

background fetching will be more used for refreshing data when they already exists.

It should not be one or the other. We should find a way to make them live together.

Maybe that, by default, background fetching should respect the fetch policy. If it returns false, then we should not run the query.

After that, we can create a plugin to force the background fetch, for instance, when giving the focus or something.

What is your opinion about this feature?

[trollepierre]

After reconsideration:

I see 3 different features that can be totally independant:

• fetchPolicies, exactly what you define above

• backgroundFetching that handles queryState.fetchStatus + isFetching, to prevent the Spinner and the bad UX. Implemented in this PR, if I revert that change line 917 in CozyClient.js and update this test expect(fetchPolicy).toHaveBeenCalled()

• forcingRefetch - or something called refetchOnWindowFocus in reactQuery, when giving the focus, when an app needs to refetch whatever the fetch policies.

I am pretty sure, the background fetching will make the fetchPolicies decrease. Maybe the forcingRefetch is not that urgent. Personally I think we should try to see how the 2 first features will collaborate, after considering implementing the forcingRefetch.

So I agree 100% with you.

[Crash--]

Nice! I'll try it on a cozy app!