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

Improve processing of options.nextFetchPolicy #8465

Merged
merged 11 commits into from
Jul 9, 2021
Merged
Prev Previous commit
Next Next commit
Specify which ObservableQuery operations run independently.
ObservableQuery reobserve operations can either modify this.options and
replace this.concast with a Concast derived from those modified options,
or they can use a (shallow) copy of this.options to create a disposable
Concast just for the current reobserve operation, without permanently
altering the configuration of the ObservableQuery.

This commit clarifies which operations receive fresh options (and do not
modify this.concast): NetworkStatus.{refetch,poll,fetchMore}, as decided
in one place, by the ObservableQuery#reobserve method.

This list is subject to change, and I suspect we may end up wanting to
use criteria besides NetworkStatus in some cases. For now, NetworkStatus
seems to capture the cases we care about.

Since NetworkStatus.poll operations "run independently" now, with their
own shallow copy of this.options, the forced fetchPolicy: "network-only"
no longer needs to be reset by nextFetchPolicy. More generally, since
polling no longer modifies the ObservableQuery options at all, the
interaction betwee polling and other kinds of network fetches should be
less complicated after this commit.
  • Loading branch information
benjamn committed Jul 9, 2021
commit 143e2ea29c0f1eeffbee7decc5778291ab6bd770
44 changes: 28 additions & 16 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,6 @@ once, rather than every time you call fetchMore.`);
if (!isNetworkRequestInFlight(this.queryInfo.networkStatus)) {
this.reobserve({
fetchPolicy: "network-only",
nextFetchPolicy: this.options.fetchPolicy || "cache-first",
}, NetworkStatus.poll).then(poll, poll);
} else {
poll();
Expand Down Expand Up @@ -622,26 +621,38 @@ once, rather than every time you call fetchMore.`);
newNetworkStatus?: NetworkStatus,
): Promise<ApolloQueryResult<TData>> {
this.isTornDown = false;
let options: WatchQueryOptions<TVariables, TData>;
if (newNetworkStatus === NetworkStatus.refetch) {
options = Object.assign({}, this.options, compact(newOptions));
} else {
if (newOptions) {
Object.assign(this.options, compact(newOptions));
}

const useDisposableConcast =
// Refetching uses a disposable Concast to allow refetches using different
// options/variables, without permanently altering the options of the
// original ObservableQuery.
newNetworkStatus === NetworkStatus.refetch ||
// The fetchMore method does not actually call the reobserve method, but,
// if it did, it would definitely use a disposable Concast.
newNetworkStatus === NetworkStatus.fetchMore ||
// Polling uses a disposable Concast so the polling options (which force
// fetchPolicy to be "network-only") won't override the original options.
newNetworkStatus === NetworkStatus.poll;

const options = useDisposableConcast
// Disposable Concast fetches receive a shallow copy of this.options
// (merged with newOptions), leaving this.options unmodified.
? compact(this.options, newOptions)
: Object.assign(this.options, compact(newOptions));

// We can skip calling updatePolling if we're not changing this.options.
if (!useDisposableConcast) {
this.updatePolling();
options = this.options;
}

const concast = this.fetch(options, newNetworkStatus);
if (newNetworkStatus !== NetworkStatus.refetch) {
// 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 (!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) {
this.concast.removeObserver(this.observer, true);
}
Expand All @@ -650,6 +661,7 @@ once, rather than every time you call fetchMore.`);
}

concast.addObserver(this.observer);

return concast.promise;
}

Expand Down