Skip to content
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

Call this.currentObservable.tearDownQuery more consistently. #7259

Merged
merged 1 commit into from
Oct 30, 2020

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Oct 29, 2020

Follow-up to #7205, per my comment #7254 (comment).

Instead of tearing down queryData.currentObservable only when queryData.getOptions().skip is true in removeQuerySubscription (which was the idea of #7205), I split the tear-down functionality into a separate private method (removeObservable), which works regardless of the skip status, and is called everywhere removeQuerySubscription is called, except in resubscribeToQuery, where we specifically want to preserve this.observableQuery.

Follow-up to #7205, per my comment in #7254:
#7254 (comment)

Instead of tearing down this.currentObservable only when
this.getOptions().skip is true in QueryData#removeQuerySubscription, I
split the tear-down functionality into a separate private method
(QueryData#removeObservable), which is now called everywhere
removeQuerySubscription is called, except in resubscribeToQuery, where we
specifically want to preserve this.observableQuery.
@@ -63,6 +63,7 @@ export class QueryData<TData, TVariables> extends OperationData {
const { skip, query } = this.getOptions();
if (skip || query !== this.previous.query) {
this.removeQuerySubscription();
this.removeObservable(!skip);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When skip is true, this code passes false to this.removeObservable, so that this.currentObservable will not be deleted after being torn down, so we can restart it later (once skip is no longer true).

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @benjamn - thanks!

@benjamn benjamn merged commit d14cf0e into release-3.3 Oct 30, 2020
@benjamn benjamn deleted the call-tearDownQuery-more-consistently branch October 30, 2020 20:28
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants