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

Why is there no fetchElseReturnCacheData policy? #392

Closed
ValCanBuild opened this issue Oct 19, 2018 · 14 comments
Closed

Why is there no fetchElseReturnCacheData policy? #392

ValCanBuild opened this issue Oct 19, 2018 · 14 comments
Labels
caching enhancement Issues outlining new things we want to do or things that will make our lives as devs easier networking-stack
Milestone

Comments

@ValCanBuild
Copy link

ValCanBuild commented Oct 19, 2018

Hello, first time user of Apollo-iOS here, though I have been using Apollo android for quite some time.

In Apollo Android there is NetworkFirst ResponseFetcher (that I assume to be an equivalent of Apollo.CachePolicy in iOS). The description of this fetcher is

  /**
   * Signals the apollo client to first fetch the data from the network. If network request fails, then the
   * data is fetched from the normalized cache. If the data is not present in the normalized cache, then the
   * exception which led to the network request failure is rethrown.
   */

This has been very useful to me in Android for showing good offline state when calls fail. I was wondering why there isn't an equivalent functionality in iOS. Is it on purpose or has it just not been requested?

@designatednerd
Copy link
Contributor

Good question! I think it's just not been implemented. I think this would be a solid enhancement though. I'll put it on the list, but it's a bit of a long list so I don't have an ETA for you.

@designatednerd designatednerd added enhancement Issues outlining new things we want to do or things that will make our lives as devs easier caching labels Jul 15, 2019
@designatednerd
Copy link
Contributor

So after some looking into this, there is the returnCacheDataAndFetch option for caching. This immediately returns cached data and then attempts to fetch fresh data from the server. That's a bit of the reverse of what you were discussing, but it has the same effect of "If the fetch from the server fails, still show whatever local data exists, and then if both fail show the server error".

Does that cover the functionality you're looking for here, or is there a benefit I'm missing to trying the fetch first?

@ValCanBuild
Copy link
Author

@designatednerd returnCacheDataAndFetch will first show cached data to the user and then update with the new one. I don't want that behaviour. I rather show a loading state, while the new data is fetched, and only fallback to the old data if fetching the new one fails.

Does that make sense?

@designatednerd
Copy link
Contributor

The what definitely makes sense - I'm not quite clear on why that's a better user experience than starting with old data and refreshing. I think getting a better understanding of that would probably help me prioritize this.

@ValCanBuild
Copy link
Author

I agree that in general that's a better user experience but I'm currently adopting requirements from an android app that behaves in this way which is why I requested the feature. I'm currently doing this myself using RxSwift but i rather have native support.

Sorry i can't give you a better reason :(

@designatednerd
Copy link
Contributor

OK - that helps though, the fact that it's trying to match an android experience with an option that doesn't exist on iOS. I'll see what I can do, though I don't have a timeline for you, I've got some other stuff I'd like to get shipped this week.

@pgawlowski
Copy link

@ValCanBuild - can you share Your current approach with RxSwift?

I would alos love to see this feature in a native way.

@ValCanBuild
Copy link
Author

@pgawlowski I've not checked this code still runs against latest Swift and Apollo versions but it went something like this:

func fetch<Query: GraphQLQuery>(_ query: Query, cachePolicy: CachePolicy = Apollo.CachePolicy.fetchIgnoringCacheData) -> Single<Query.Data> {
        return Single.deferred { [unowned self] in
            self.apollo.rxfetch(query: query, cachePolicy: cachePolicy)
        }
        .catchError { [unowned self] in
            let retryFetchFromCache = $0.isConnectivityError && cachePolicy == Apollo.CachePolicy.fetchIgnoringCacheData
            if retryFetchFromCache {
                return self.apollo.rxfetch(query: query, cachePolicy: Apollo.CachePolicy.returnCacheDataElseFetch)
            } else {
                return .error($0)
            }
        }
    }

@designatednerd
Copy link
Contributor

Thanks for the example in RxSwift.

I'm looking at some changes to the networking stack now, I'll have to think on making this official since it's a totally different order than our other operations, but the changes would at least allow you to build this yourself reasonably easily.

@pgawlowski
Copy link

@designatednerd - I can't promise (cause I am already overloaded with work) but I might be able to prepare some PR here, since I need this feature :)

I've already checked the source code and I think this feature might be easy to introduce.

@designatednerd
Copy link
Contributor

Hey @pgawlowski I'm gonna have an RFC up for a new network architecture sometime this week or next - you may want to hold off until that's up since it'll change how this all works.

@designatednerd
Copy link
Contributor

@pgawlowski @ValCanBuild Please check out the RFC for networking changes at #1340 - we're not going to add this as a default option, but it should be fairly simple to reorder interceptors in order to facilitate the same idea.

@designatednerd
Copy link
Contributor

OK, getting ready to beta this but you should be able to do this with custom ordering of the provided interceptors and/or a custom interceptor. Please see #1341 for the current implementation.

@designatednerd designatednerd added this to the Networking Beta milestone Sep 9, 2020
@designatednerd
Copy link
Contributor

The networking stack is now available as a beta at 0.33.0-beta1 (available via the betas/networking-stack branch and through PR #1386). Going to close this out, please open a new issue if your problems aren't addressed by the new networking stacks.

@designatednerd designatednerd modified the milestones: Networking Beta, Next Release Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
caching enhancement Issues outlining new things we want to do or things that will make our lives as devs easier networking-stack
Projects
None yet
Development

No branches or pull requests

3 participants