Skip to content

Commit

Permalink
Capture options.variables in ObservableQuery#reobserve.
Browse files Browse the repository at this point in the history
This resolves the concern I raised in this comment:
#8642 (comment)
  • Loading branch information
benjamn committed Aug 16, 2021
1 parent e1a1ceb commit c834089
Showing 1 changed file with 55 additions and 29 deletions.
84 changes: 55 additions & 29 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ export interface UpdateQueryOptions<TVariables> {

let warnedAboutUpdateQuery = false;

interface Last<TData, TVars> {
interface Last<TData, TVariables> {
result: ApolloQueryResult<TData>;
variables: TVars;
variables?: TVariables;
error?: ApolloError;
}

Expand Down Expand Up @@ -78,7 +78,11 @@ export class ObservableQuery<

private queryInfo: QueryInfo;

// When this.concast is defined, this.observer is the Observer currently
// subscribed to that Concast.
private concast?: Concast<ApolloQueryResult<TData>>;
private observer?: Observer<ApolloQueryResult<TData>>;

private pollingInfo?: {
interval: number;
timeout: ReturnType<typeof setTimeout>;
Expand Down Expand Up @@ -611,13 +615,16 @@ once, rather than every time you call fetchMore.`);
poll();
}

private updateLastResult(newResult: ApolloQueryResult<TData>) {
private updateLastResult(
newResult: ApolloQueryResult<TData>,
variables = this.variables,
) {
this.last = {
...this.last,
result: this.queryManager.assumeImmutableResults
? newResult
: cloneDeep(newResult),
variables: { ...this.variables } as TVariables,
variables: variables && { ...variables },
};
if (!isNonEmptyArray(newResult.errors)) {
delete this.last.error;
Expand Down Expand Up @@ -672,21 +679,30 @@ once, rather than every time you call fetchMore.`);
}

const concast = this.fetch(options, newNetworkStatus);
const observer: Observer<ApolloQueryResult<TData>> = {
next: result => {
this.reportResult(result, options.variables);
},
error: error => {
this.reportError(error, options.variables);
},
};

if (!useDisposableConcast) {
// We use the {add,remove}Observer methods directly to avoid wrapping
// observer with an unnecessary SubscriptionObserver object, in part so
// that we can remove it here without triggering any unsubscriptions,
// because we just want to ignore the old observable, not prematurely shut
// it down, since other consumers may be awaiting this.concast.promise.
if (this.concast) {
if (this.concast && this.observer) {
this.concast.removeObserver(this.observer, true);
}

this.concast = concast;
this.observer = observer;
}

concast.addObserver(this.observer);
concast.addObserver(observer);

return concast.promise;
}
Expand All @@ -698,41 +714,51 @@ once, rather than every time you call fetchMore.`);
// save the fetchMore result as this.lastResult, causing it to be
// ignored due to the this.isDifferentFromLastResult check in
// this.observer.next.
this.observer.next(this.getCurrentResult(false));
this.reportResult(
this.getCurrentResult(false),
this.variables,
);
}

private observer = {
next: (result: ApolloQueryResult<TData>) => {
if (this.getLastError() || this.isDifferentFromLastResult(result)) {
this.updateLastResult(result);
iterateObserversSafely(this.observers, 'next', result);
}
},
private reportResult(
result: ApolloQueryResult<TData>,
variables: TVariables | undefined,
) {
if (this.getLastError() || this.isDifferentFromLastResult(result)) {
this.updateLastResult(result, variables);
iterateObserversSafely(this.observers, 'next', result);
}
}

error: (error: ApolloError) => {
// Since we don't get the current result on errors, only the error, we
// must mirror the updates that occur in QueryStore.markQueryError here
this.updateLastResult({
...this.getLastResult(),
error,
errors: error.graphQLErrors,
networkStatus: NetworkStatus.error,
loading: false,
} as ApolloQueryResult<TData>);

iterateObserversSafely(this.observers, 'error', this.last!.error = error);
},
};
private reportError(
error: ApolloError,
variables: TVariables | undefined,
) {
// Since we don't get the current result on errors, only the error, we
// must mirror the updates that occur in QueryStore.markQueryError here
const errorResult = {
...this.getLastResult(),
error,
errors: error.graphQLErrors,
networkStatus: NetworkStatus.error,
loading: false,
} as ApolloQueryResult<TData>;

this.updateLastResult(errorResult, variables);

iterateObserversSafely(this.observers, 'error', this.last!.error = error);
}

public hasObservers() {
return this.observers.size > 0;
}

private tearDownQuery() {
if (this.isTornDown) return;
if (this.concast) {
if (this.concast && this.observer) {
this.concast.removeObserver(this.observer);
delete this.concast;
delete this.observer;
}

this.stopPolling();
Expand Down

0 comments on commit c834089

Please sign in to comment.