Add re-fetch query watchers #509
Conversation
Re-fetch query watchers after mutation completed Refactor ApolloTracker. Closes apollographql#452
| }); | ||
| } else { //noinspection unchecked | ||
| responseCallback.onResponse(response.parsedResponse.get()); | ||
| private void refetchQueryWatchers() { |
There was a problem hiding this comment.
Although strange, should this be refetchRefetchQueryWatchers?
There was a problem hiding this comment.
Sorry what do you mean?
There was a problem hiding this comment.
'refetchQueryWatchers` sort of makes it seem like we will be refetching all query watchers - but really we are refetching the set of "refetch" query watchers, right?
It's an internal method, so not a big deal at all.
| queryFetcher.get().refetchAsync(new QueryFetcher.OnFetchCompleteCallback() { | ||
| @Override public void onFetchComplete() { | ||
| //noinspection unchecked | ||
| originalCallback.onResponse(response.parsedResponse.get()); |
There was a problem hiding this comment.
Would it be safe to not block the original call on the refetch queries? I can see the argument for both ways, ideally if it blocked or not would be user configurable.
Additionally, the actually refetch routine seems to be serially executed. Maybe we should investigate just dumping all the refetch querries into one graphql query? Although is may complicate query whitelisting.
There was a problem hiding this comment.
Maybe we should investigate just dumping all the refetch querries into one graphql query?
Yeah that will be awesome, we def need to explore this, I see though as it won't be trivial. At the same time we can explore overall merging queries (smth like throttling ) into one call.
There was a problem hiding this comment.
LGTM overall. Have some concerns with the fact that refetch queries seem to block the main response, although I can see why that may be desired in some use cases.
Also, do we need to guard against an infinite loop of refetch queries? (Two refetch queries reference each other and ping pong back and forth refetching)
That should not be the case, as refetch queries names can be set only for
Yeah I thought about this and don't have strong opinion on that so not sure what the right way. |
A better approach would be to use transport batching probably (sending multiple documents/query IDs in one request), that way you can still whitelist, but they can be sent in one request and batched on the server. |
|
@digitalbuddha any comments? |
Re-fetch query watchers after mutation completed
Refactor ApolloTracker.
Closes #452