-
Notifications
You must be signed in to change notification settings - Fork 675
Add cacheInterceptor() and autoPersistedQueriesInterceptor() #6455
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
Conversation
// Removing existing interceptors added for configuring an [ApolloStore]. | ||
// If a builder is reused from an existing client using `newBuilder()` and we try to configure a new `store()` on it, we first need to | ||
// remove the old interceptors. | ||
val storeInterceptors = interceptors.filterIsInstance<ApolloStoreInterceptor>() | ||
storeInterceptors.forEach { | ||
removeInterceptor(it) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not an issue anymore!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the ApolloStoreInterceptor
interface can also be deleted, then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call! → #6612
✅ Docs preview has no changesThe preview was not built because there were no changes. Build ID: 423ee57de2d9619f11115d73 |
@@ -228,8 +259,8 @@ internal fun <D : Query.Data> ApolloCall<D>.watchInternal(data: D?): Flow<Apollo | |||
|
|||
val ApolloClient.apolloStore: ApolloStore | |||
get() { | |||
return interceptors.firstOrNull { it is ApolloCacheInterceptor }?.let { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this no longer works, and should be cacheInterceptor
(but then it must be made public)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but then it must be made public
I have made it public
return chain.proceed(request).map { response -> | ||
response.newBuilder().cacheHeaders(CacheHeaders.Builder().addHeader(ApolloCacheHeaders.DO_NOT_STORE, "").build()).build() | ||
} | ||
return chain.proceed( | ||
request.newBuilder() | ||
.cacheHeaders( | ||
CacheHeaders.Builder().addHeader(ApolloCacheHeaders.DO_NOT_STORE, "").build() | ||
) | ||
.build() | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yet another good side effect IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* Add cacheInterceptor() and autoPersistedQueriesInterceptor() * make cacheInterceptor public * add mention in the changelog * remove obsolete tests * update the test to include patching based on the response * update README (cherry picked from commit 1a9918a)
…6456) * Add cacheInterceptor() and autoPersistedQueriesInterceptor() * make cacheInterceptor public * add mention in the changelog * remove obsolete tests * update the test to include patching based on the response * update README (cherry picked from commit 1a9918a) Co-authored-by: Martin Bonnin <martin@mbonnin.net>
See apollographql/apollo-kotlin-normalized-cache#123 (comment)