-
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
Support ApolloClient#stop for safe client disposal. #4336
Conversation
Inspired by apollographql/react-apollo#2738, it should be possible to shut down an ApolloClient instance without making assumptions about its implementation details. This client.stop() method should also be useful for server-side rendering, where you're supposed to create (and throw away) a new ApolloClient instance for each request. I'm not entirely sure this implementation cleans up everything that might need to be cleaned up, but I am sure it's better than nothing.
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.
Looks great @benjamn! This should definitely help. I know we're moving quick to address apollographql/react-apollo#2738, but we'll want to circle back here to get some tests and API docs changes in place. I'll make a note. Thanks!
This method was introduced in apollo-client@2.4.12: apollographql/apollo-client#4336
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.
// After calling this.stopPollingQuery for all registered queries, calling | ||
// fetchQueriesOnInterval will remove the corresponding intervals. | ||
Object.keys(this.intervalQueries).forEach(interval => { | ||
this.fetchQueriesOnInterval(+interval); |
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.
@benjamn Why is there a +
before interval?
Awesome! |
This method was introduced in apollo-client@2.4.12: apollographql/apollo-client#4336
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.
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.
Inspired by apollographql/react-apollo#2738, it should be possible to shut down an
ApolloClient
instance without making assumptions about its implementation details.This
client.stop()
method should also be useful for server-side rendering, where you're supposed to create (and throw away) a newApolloClient
instance for each request.I'm not entirely sure this implementation cleans up everything that might need to be cleaned up, but I am sure it's better than nothing.