Skip to content

Commit

Permalink
Avoid deleting options.nextFetchPolicy after applying it.
Browse files Browse the repository at this point in the history
Despite my vague suggestion in the removed comments that deleting
options.nextFetchPolicy somehow helped make the fetchPolicy transition
idempotent (a concern that only matters when nextFetchPolicy is a
function), leaving options.nextFetchPolicy intact should also be
safe/idempotent in virtually all cases, including every case where
nextFetchPolicy is just a string, rather than a function.

More importantly, leaving options.nextFetchPolicy intact allows it to be
applied more than once, which seems to fix issue #6839.
  • Loading branch information
benjamn committed Jul 9, 2021
1 parent 4e6c1c2 commit add15f5
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 8 deletions.
4 changes: 1 addition & 3 deletions src/__tests__/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3319,7 +3319,6 @@ describe('@connection', () => {
// again to perform an additional transition.
this.nextFetchPolicy = fetchPolicy => {
++nextFetchPolicyCallCount;
expect(fetchPolicy).toBe("cache-and-network");
return "cache-first";
};
return "cache-and-network";
Expand Down Expand Up @@ -3372,9 +3371,8 @@ describe('@connection', () => {
client.cache.evict({ fieldName: "count" });
} else if (handleCount === 6) {
expect(result.data).toEqual({ count: 2 });
expect(nextFetchPolicyCallCount).toBe(3);
expect(nextFetchPolicyCallCount).toBe(4);
expect(obs.options.fetchPolicy).toBe("cache-first");
expect(obs.options.nextFetchPolicy).toBeUndefined();
setTimeout(resolve, 50);
} else {
reject("too many results");
Expand Down
5 changes: 0 additions & 5 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -732,11 +732,6 @@ export function applyNextFetchPolicy<TData, TVars>(
} = options;

if (nextFetchPolicy) {
// The options.nextFetchPolicy transition should happen only once, but it
// should also be possible (though uncommon) for a nextFetchPolicy function
// to set options.nextFetchPolicy to perform an additional transition.
options.nextFetchPolicy = void 0;

// When someone chooses "cache-and-network" or "network-only" as their
// initial FetchPolicy, they often do not want future cache updates to
// trigger unconditional network requests, which is what repeatedly
Expand Down

0 comments on commit add15f5

Please sign in to comment.