-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Improve standalone client.refetchQueries method to support automatic detection of queries needing to be refetched. #8000
Conversation
if (typeof removeOptimistic === "string") { | ||
this.optimisticData = this.optimisticData.removeLayer(removeOptimistic); | ||
} |
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.
The removeOptimistic
option is a powerful but easy-to-miss new feature of cache.batch
, since it allows us to remove optimistic mutation layers at the same time (within the same batch
operation) as the final mutation update
, triggering only one broadcastWatches
call for both the removal and the update
. Previously, optimistic layers were removed separately, after the final update
, which could trigger more than one broadcast (likely harmless but potentially noisy).
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.
Likely more review tomorrow but I’m punching out for the day.
20d6c78
to
526869f
Compare
26639f0
to
19d357f
Compare
Previously, removing an optimistic EntityStore layer with removeOptimistic would dirty all fields of any StoreObjects contained by the removed layer, ignoring the possibility that some of those field values might have been inherited as-is from the parent layer. With this commit, we dirty only those fields whose values will be observably changed by removing the layer, which requires comparing field values between the layer-to-be-removed and its parent layer.
Notice that the type of `results` in client.refetchQueries(...).then(results => ...) is correctly inferred to be ApolloQueryResult<any>[], because the onQueryUpdated function returned true, which triggers a refetch, which returns a Promise<ApolloQueryResult<any>>.
318039c
to
512804c
Compare
@brainkim @timbotnik @hwillson I added support for including queries by client.refetchQueries({
include: [queryObject],
onQueryUpdated(obs, diff) {
return obs.refetch();
},
}) I'd like to merge this PR soon so I can start working on the docs (which aren't rendering on this branch), so let me know soonish if you have any additional feedback here. Thanks! |
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.
lgtm 🎉
As promised in #7878 (comment) and #7827 (comment), this PR turns the
client.refetchQueries
method into a one-stop solution for refetching queries (without having to perform a mutation), tying together several previous ways of triggering refetches into a single, flexible API.This is the last major piece of new functionality that we plan to add to Apollo Client v3.4, and is intended to complement similar functionality added to mutations (
options.onQueryUpdated
, previously calledoptions.reobserveQuery
) in #7827, and builds on thecache.batch
primitive added toInMemoryCache
in #7819. If this PR is merged, it should conclude the implementation of the following item from the Apollo Client v3.4ROADMAP.md
section:updateQueries
,refetchQueries
, andawaitRefetchQueries
in a lot of cases.Since the API has changed since I last sketched it out, here's an overview of the current functionality:
All of these options (
updateCache
,include
,onQueryUpdated
, andoptimistic
) are optional, but you have to pass eitherupdateCache
orinclude
to get any queries to be refetched byclient.refetchQueries
.This PR is a work-in-progress because this still needs more tests and documentation, but I wanted to share my progress so far, so folks can share their thoughts on this new
client.refetchQueries
API.