Skip to content

Commit

Permalink
Simplify fetchMore loading result simulation.
Browse files Browse the repository at this point in the history
PR #6221 began enforcing fetch policies more consistently/literally in
response to cache updates (so we do not accidentally skip network requests
that are required by policy).

That consistency has been valuable, but there are some exceptions where we
want to deliver a single result from the cache, but we do not want to use
the fetchQueryObservable system to deliver it, because we do not want the
delivery to have the side effect of triggering a network request under any
circumstances, regardless of the fetch policy.

For example, we want to deliver a { loading: true, networkStatus:
NetworkStatus.fetchMore } result for the original query at the beginning
of a fetchMore request (see #6583), but that one-off result should not
cause the original query to fire a network request if the cache data
happens to be incomplete, because that would likely interfere with the
fetchMore network request.

At the time I implemented #6583, it was the only exception that I knew of,
so I kept things simple and fetchMore-specific. I've since found another
example (not fetchMore-related this time), so I think it's time to make
this pattern more official. Hence ObservableQuery#observe, whose name is
meant to draw a contrast with the ObservableQuery#reobserve method. The
observe method simply delivers the latest result known to the
ObservableQuery, reading from the cache if necessary, whereas reobserve
reapplies the chosen fetch policy, possibly making network requests.
  • Loading branch information
benjamn committed Aug 17, 2020
1 parent c7b3ccd commit 1dd9301
Showing 1 changed file with 19 additions and 32 deletions.
51 changes: 19 additions & 32 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export class ObservableQuery<
});
}

public getCurrentResult(): ApolloQueryResult<TData> {
public getCurrentResult(saveAsLastResult?: boolean): ApolloQueryResult<TData> {
const { lastResult, lastError } = this;
const networkStatus = this.queryInfo.networkStatus || NetworkStatus.ready;
const result: ApolloQueryResult<TData> = {
Expand Down Expand Up @@ -176,7 +176,9 @@ export class ObservableQuery<
}
}

this.updateLastResult(result);
if (saveAsLastResult !== false) {
this.updateLastResult(result);
}

return result;
}
Expand Down Expand Up @@ -276,36 +278,11 @@ export class ObservableQuery<

const qid = this.queryManager.generateQueryId();

// Simulate a loading result for the original query with
// result.networkStatus === NetworkStatus.fetchMore.
if (combinedOptions.notifyOnNetworkStatusChange) {
const currentResult = this.getCurrentResult();

// If we neglect to update queryInfo.networkStatus here,
// getCurrentResult may return a loading:false result while
// fetchMore is in progress, since getCurrentResult also consults
// queryInfo.networkStatus. Note: setting queryInfo.networkStatus
// to an in-flight status means that QueryInfo#shouldNotify will
// return false while fetchMore is in progress, which is why we
// call this.reobserve() explicitly in the .finally callback after
// fetchMore (below), since the cache write will not automatically
// trigger a notification, even though it does trigger a cache
// broadcast. This is a good thing, because it means we won't see
// intervening query notifications while fetchMore is pending.
this.queryInfo.networkStatus = NetworkStatus.fetchMore;

// Simulate a loading result for the original query with
// networkStatus === NetworkStatus.fetchMore.
this.observer.next!({
// Note that currentResult is an ApolloCurrentQueryResult<TData>,
// whereas this.observer.next expects an ApolloQueryResult<TData>.
// Fortunately, ApolloCurrentQueryResult is a subtype of
// ApolloQueryResult (with additional .error and .partial fields),
// so TypeScript has no problem with this sleight of hand.
// TODO Consolidate these two types into a single type (most
// likely just ApolloQueryResult) after AC3 is released.
...currentResult,
loading: true,
networkStatus: NetworkStatus.fetchMore,
});
this.observe();
}

return this.queryManager.fetchQuery(
Expand Down Expand Up @@ -606,8 +583,18 @@ once, rather than every time you call fetchMore.`);
return this.getReobserver().reobserve(newOptions, newNetworkStatus);
}

private observer: Observer<ApolloQueryResult<TData>> = {
next: result => {
// Pass the current result to this.observer.next without applying any
// fetch policies, bypassing the Reobserver.
public observe() {
// Passing false is important so that this.getCurrentResult doesn't
// 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));
}

private observer = {
next: (result: ApolloQueryResult<TData>) => {
if (this.lastError || this.isDifferentFromLastResult(result)) {
this.updateLastResult(result);
iterateObserversSafely(this.observers, 'next', result);
Expand Down

0 comments on commit 1dd9301

Please sign in to comment.