Un-revert "Improve (and shorten) query polling implementation. (#4243)" #4337
Conversation
This comment has been minimized.
This comment has been minimized.
benjamn
force-pushed
the
un-revert-QueryScheduler-removal
branch
from
47ac099 to
3e8d0e8
Compare
| } | ||
|
|
||
| private stopQueryInStoreNoBroadcast(queryId: string) { | ||
| this.stopPollingQuery(queryId); |
There was a problem hiding this comment.
It seems pretty obvious that we should stop polling a query when we call stopQuery for that query.
| @@ -955,7 +960,12 @@ export class QueryManager<TStore> { | |||
| } | |||
|
|
|||
| public stopQuery(queryId: string) { | |||
| this.stopQueryInStore(queryId); | |||
| this.stopQueryNoBroadcast(queryId); | |||
| this.broadcastQueries(); | |||
There was a problem hiding this comment.
This now waits to call this.broadcastQueries() until after the query has been removed, whereas previously this.stopQueryInStore(queryId) would stop the query, call this.broadcastQueries(), then return control to stopQuery, which would finally call this.removeQuery(queryId).
It makes more sense to me to wait until the query has been removed to broadcast queries, but I'm not sure if this semantic subtlety was important before.
|
@hwillson I still think this should be part of a minor version bump for |
|
@benjamn Agreed! Let's get this out with 2.5.0. Thanks! |
This reverts commit 9739ff6. Now that we have a uniform interface for terminating ApolloClient instances (#4336), there should be no need for any external code to access the QueryScheduler abstraction, which this commit removes. We should wait to merge and release this change until after apollographql/react-apollo#2741 has been merged and released, so that we don't break older versions of MockedProvider.
benjamn
force-pushed
the
un-revert-QueryScheduler-removal
branch
from
9eba27e to
a94c2aa
Compare
This reverts commit 9739ff6.
Now that we have a uniform interface for terminating
ApolloClientinstances (#4336), there should be no need for any external code to access theQuerySchedulerabstraction, which this commit removes.We should wait to merge and release this change until after apollographql/react-apollo#2741 has been merged and released, so that we don't break older versions of
MockedProvider.