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

Revert PR #6353 and support options.nextFetchPolicy instead. #6712

Merged
merged 2 commits into from
Jul 27, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,28 +219,33 @@ export class ObservableQuery<
));
}

const reobserveOptions: Partial<WatchQueryOptions<TVariables>> = {
// Always disable polling for refetches.
pollInterval: 0,
};

// Unless the provided fetchPolicy always consults the network
// (no-cache, network-only, or cache-and-network), override it with
// network-only to force the refetch for this fetchQuery call.
if (fetchPolicy !== 'no-cache' &&
fetchPolicy !== 'cache-and-network') {
fetchPolicy = 'network-only';
reobserveOptions.fetchPolicy = 'network-only';
// Go back to the original options.fetchPolicy after this refetch.
reobserveOptions.nextFetchPolicy = fetchPolicy;
}

if (variables && !equal(this.options.variables, variables)) {
// Update the existing options with new variables
this.options.variables = {
reobserveOptions.variables = this.options.variables = {
...this.options.variables,
...variables,
} as TVariables;
}

return this.newReobserver(false).reobserve({
fetchPolicy,
variables: this.options.variables,
// Always disable polling for refetches.
pollInterval: 0,
}, NetworkStatus.refetch);
return this.newReobserver(false).reobserve(
reobserveOptions,
NetworkStatus.refetch,
);
}

public fetchMore<K extends keyof TVariables>(
Expand Down
28 changes: 13 additions & 15 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -909,22 +909,20 @@ export class QueryManager<TStore> {
concast.cleanup(() => {
this.fetchCancelFns.delete(queryId);

if (fetchPolicy === "cache-and-network" ||
fetchPolicy === "network-only") {
if (options.nextFetchPolicy) {
// When someone chooses cache-and-network or network-only as their
// initial FetchPolicy, they almost certainly do not want future cache
// updates to trigger unconditional network requests, which is what
// repeatedly applying the cache-and-network or network-only policies
// would seem to require. Instead, when the cache reports an update
// after the initial network request, subsequent network requests should
// be triggered only if the cache result is incomplete. This behavior
// corresponds exactly to switching to a cache-first FetchPolicy, so we
// modify options.fetchPolicy here for the next fetchQueryObservable
// call, using the same options object that the Reobserver always passes
// to fetchQueryObservable. Note: if these FetchPolicy transitions get
// much more complicated, we might consider using some sort of state
// machine to capture the transition rules.
options.fetchPolicy = "cache-first";
// initial FetchPolicy, they often do not want future cache updates to
// trigger unconditional network requests, which is what repeatedly
// applying the cache-and-network or network-only policies would seem
// to imply. Instead, when the cache reports an update after the
// initial network request, it may be desirable for subsequent network
// requests to be triggered only if the cache result is incomplete.
// The options.nextFetchPolicy option provides an easy way to update
// options.fetchPolicy after the intial network request, without
// having to call observableQuery.setOptions.
options.fetchPolicy = options.nextFetchPolicy;
// The options.nextFetchPolicy transition should happen only once.
options.nextFetchPolicy = void 0;
}
});

Expand Down
18 changes: 13 additions & 5 deletions src/core/__tests__/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -986,8 +986,7 @@ describe('ObservableQuery', () => {
// Although the options.fetchPolicy we passed just now to
// fetchQueryByPolicy should have been network-only,
// observable.options.fetchPolicy should now be updated to
// cache-first, since network-only (and cache-and-network) fetch
// policies fall back to cache-first after the first request.
// cache-first, thanks to options.nextFetchPolicy.
expect(observable.options.fetchPolicy).toBe('cache-first');
const fqoCalls = mocks.fetchQueryObservable.mock.calls;
expect(fqoCalls.length).toBe(2);
Expand Down Expand Up @@ -1206,17 +1205,26 @@ describe('ObservableQuery', () => {
observable.refetch().then(result => {
expect(result).toEqual({
data: {
counter: 3,
counter: 5,
name: 'Ben',
},
loading: false,
networkStatus: NetworkStatus.ready,
});
resolve();
setTimeout(resolve, 50);
}, reject);
},
);
} else if (handleCount > 2) {
} else if (handleCount === 3) {
expect(result).toEqual({
data: {
counter: 3,
name: 'Ben',
},
loading: true,
networkStatus: NetworkStatus.refetch,
});
} else if (handleCount > 3) {
reject(new Error('should not get here'));
}
},
Expand Down
6 changes: 5 additions & 1 deletion src/core/watchQueryOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,13 @@ export interface WatchQueryOptions<TVariables = OperationVariables>
extends QueryBaseOptions<TVariables>,
ModifiableWatchQueryOptions<TVariables> {
/**
* Specifies the {@link FetchPolicy} to be used for this query
* Specifies the {@link FetchPolicy} to be used for this query.
*/
fetchPolicy?: WatchQueryFetchPolicy;
/**
* Specifies the {@link FetchPolicy} to be used after this query has completed.
*/
nextFetchPolicy?: WatchQueryFetchPolicy;
}

export interface FetchMoreQueryOptions<TVariables, K extends keyof TVariables> {
Expand Down
5 changes: 4 additions & 1 deletion src/react/hoc/__tests__/queries/loading.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,10 @@ describe('[queries] loading', () => {
let count = 0;

const Container = graphql<{}, Data>(query, {
options: { fetchPolicy: 'network-only' }
options: {
fetchPolicy: 'network-only',
nextFetchPolicy: 'cache-first',
},
})(
class extends React.Component<ChildProps<{}, Data>> {
render() {
Expand Down
1 change: 1 addition & 0 deletions src/react/hoc/__tests__/queries/skip.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,7 @@ describe('[queries] skip', () => {
const Container = graphql<any>(query, {
options: {
fetchPolicy: 'network-only',
nextFetchPolicy: 'cache-first',
notifyOnNetworkStatusChange: true
},
skip: ({ skip }) => skip
Expand Down
1 change: 1 addition & 0 deletions src/react/types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export interface BaseQueryOptions<TVariables = OperationVariables> {
ssr?: boolean;
variables?: TVariables;
fetchPolicy?: WatchQueryFetchPolicy;
nextFetchPolicy?: WatchQueryFetchPolicy;
errorPolicy?: ErrorPolicy;
pollInterval?: number;
client?: ApolloClient<any>;
Expand Down