listen to cache changes when using FetchPolicy.NetworkOnly

[LiLatee]

Hi! 👋

Issue

That PR solves the problem I described here #574.

The problem is that when we use FetchPolicy.NetworkOnly and something in the cache changes (e.g. with use of writeFragment method) that relates to our query then that change will not emit a new event for the query. (That happens for CacheAndNetwork and CacheFirst policies).
I am not sure whether it should work like that, because I cannot find any reason to save the response to the cache, but do not listen for the changes in the cache.

So if there is any reason for that then please let me know. In that case, maybe it would be beneficial to add another FetchPolicy?

Solution

I added listening for changes in the cache when using FetchPolicy.NetworkOnly and added a tests explaining the usage of it.

[netlify]

✅ Deploy Preview for verdant-brigadeiros-5171fa canceled.

Name	Link
🔨 Latest commit	3eddbf4
🔍 Latest deploy log	https://app.netlify.com/sites/verdant-brigadeiros-5171fa/deploys/65c8d3a8e9c1f40008af76a6

[knaeckeKami]

I am not sure whether it should work like that, because I cannot find any reason to save the response to the cache, but do not listen for the changes in the cache.
So if there is any reason for that then please let me know. In that case, maybe it would be beneficial to add another FetchPolicy?

Yes, there are good reasons to do that.
Writing the network response to the cache will notify all other listeners of the cache for every identifiable object (per default: objects that have an id or _id field) in the response.

Say, you have PostDetails(id: postId) widget, which uses cache.watchFragment() to watch the cache for a post of the given id.

And you periodically poll for the latest posts using .NetworkOnly. If the response contains a post with the ID that is watched by the PostDetails widget, this widget will be updated automatically.

.NetworkOnly ist mostly useful to update other listeners automatically without having to do state management manually.

[LiLatee]

And you periodically poll for the latest posts using .NetworkOnly. If the response contains a post with the ID that is watched by the PostDetails widget, this widget will be updated automatically.

Yes, exactly. But isn't that something expected that you want to have newest version of the post? Why would you like to stick with the old one?

As you can remember I had a query with CacheAndNetwork policy and I was calling the same request with NetworkOnly policy in case of pull to refresh action, but then any changes in cache don't propagate.

So what do you think about adding an additional FetchPolicy type? Like 'NetworkOnlyWithUpdates'.

In case of my project I've overridden default FetchPolicyTypedLink to a custom one.

[knaeckeKami]

Yes, exactly. But isn't that something expected that you want to have newest version of the post? Why would you like to stick with the old one?

I don't quite understand. Writing the .NetworkOnly response to the cache is what causes the other listeners to update.
But I would not expect the .NetworkOnly request to listen to the cache, since, well, it is called NetworkOnly.

This is also the behaviour of Apollo, from which we adopted the FetchPolicies.

As you can remember I had a query with CacheAndNetwork policy and I was calling the same request with NetworkOnly policy in case of pull to refresh action, but then any changes in cache don't propagate.

Yes. This is quite the unfortunate behaviour of using the Refetching/ Pagination feature together with watching the cache.
It would just work though if you didn't use any requestId, use .CacheAndNetwork for the initial request and if you executed refetches /paginations using .request() with .NetworkOnly. Then, all the work would be done on the cache and not on the RequestControllerTypedLink level.
I agree that is is not ideal behaviour though. Not sure what the best way forward is. Maybe it would be time to refactor the refetching + pagination logic and handle it all in the cache.

So what do you think about adding an additional FetchPolicy type? Like 'NetworkOnlyWithUpdates'.

That's something I think is reasonable. Changing .NetworkOnly to also include cache updates would be quite breaking and unintuitive IMO.

Adding type to an enum would also be breaking (in case someone depends on exhaustive matching for example), but probably very low impact.

[LiLatee]

Maybe it would be time to refactor the refetching + pagination logic and handle it all in the cache.

Do you mean on my side or in the Ferry?

Changing .NetworkOnly to also include cache updates would be quite breaking and unintuitive IMO.

Yes, I totally agree. But for the beginning, I wanted to show the problem and possible solution, so I've modified the existing policy because it's faster than creating a new one.

Adding type to an enum would also be breaking (in case someone depends on exhaustive matching for example), but probably very low impact.

Sure, so just let me know if I should prepare a PR with the new FetchPolicy or leave it for now if you decide that's not needed.

P.S. I see that Apollo has something called nextFetchPolicy. I am not sure if it solves my problem, but could be.

[knaeckeKami]

Do you mean on my side or in the Ferry?

In Ferry. It's clear that the refetching/paging API using the requestcontroller and the cache do not play well together.
But at the moment, I don't have a clear picture on how to improve that.

Sure, so just let me know if I should prepare a PR with the new FetchPolicy or leave it for now if you decide that's not needed.
P.S. I see that Apollo has something called [nextFetchPolicy (https://www.apollographql.com/docs/react/data/queries/#nextfetchpolicy). I am not sure if it solves my problem, but could be.

Thanks! At the moment, I think a new fetch policy makes more sense. But let me take some time to review the approach of apollo.

[LiLatee]

Thanks! At the moment, I think a new fetch policy makes more sense. But let me take some time to review the approach of apollo.

Sure 😊