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

Allow different types of fetches on a watcher #1802

Merged
merged 3 commits into from
Jun 15, 2021

Conversation

gsbernstein
Copy link
Contributor

Publicly exposing GraphQLQueryWatcher's fetch() function would allow us to make other kinds of fetches besides the fetchIgnoringCacheData exposed by refetch(). Our use case is that our custom version of InMemoryNormalizedCache has cache expiration, and we want to be able to hit the network only if the cache has become invalid. Alternately, we could manually check the cache, but this seems cleaner.

Publicly exposing the fetch() function would allow us to make other kinds of fetches besides the `fetchIgnoringCacheData` exposed by `refetch()`. Our use case is that our custom version of `InMemoryNormalizedCache` has cache expiration, and we want to be able to hit the network only if the cache has become invalid. Alternately, we could manually check the cache, but this seems cleaner.
@AnthonyMDev
Copy link
Contributor

Thanks for the PR and explanation @gsbernstein. I think it might be cleaner to expose the cachePolicy as a parameter on refetch(), since the initial fetch is done for you automatically. So I'd change it to public func refetch(cachePolicy: CachePolicy = .fetchIgnoringCacheData)

@gsbernstein
Copy link
Contributor Author

In that case, should I just combine the fetch and refetch functions into a single refetch(cachePolicy: CachePolicy = .fetchIgnoringCacheData)?

@AnthonyMDev
Copy link
Contributor

Let's leave the private fetch function as is for now and just change the refetch function please!

@AnthonyMDev
Copy link
Contributor

Thanks for finishing this up!

@AnthonyMDev AnthonyMDev merged commit 0ef536d into apollographql:main Jun 15, 2021
@hwillson hwillson removed this from the MM-2021-06 milestone Jul 29, 2021
@AnthonyMDev AnthonyMDev added the include-in-changelog Indicates that changes from a PR should be noted in the changelog for their release. label Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include-in-changelog Indicates that changes from a PR should be noted in the changelog for their release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants