Skip to content

Commit

Permalink
Make refetch results overwrite existing data.
Browse files Browse the repository at this point in the history
This is how I would ideally like to fix #7491, but I'm worried it could be
a breaking change for any application code that relies on refetch results
getting merged with existing data, rather than replacing it (which is what
refetch was originally meant to do, since AC2).

I will follow this commit with one that makes this the overwrite behavior
opt-in (and thus backwards compatible).
  • Loading branch information
benjamn committed Mar 24, 2021
1 parent 9410f17 commit a16205e
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 15 deletions.
11 changes: 9 additions & 2 deletions src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ export type QueryStoreValue = Pick<QueryInfo,
| "graphQLErrors"
>;

export const enum CacheWriteBehavior {
FORBID,
OVERWRITE,
MERGE,
};

const destructiveMethodCounts = new (
canUseWeakMap ? WeakMap : Map
)<ApolloCache<any>, number>();
Expand Down Expand Up @@ -307,7 +313,7 @@ export class QueryInfo {
| "variables"
| "fetchPolicy"
| "errorPolicy">,
allowCacheWrite: boolean,
cacheWriteBehavior: CacheWriteBehavior,
) {
this.graphQLErrors = isNonEmptyArray(result.errors) ? result.errors : [];

Expand All @@ -318,7 +324,7 @@ export class QueryInfo {
if (options.fetchPolicy === 'no-cache') {
this.diff = { result: result.data, complete: true };

} else if (!this.stopped && allowCacheWrite) {
} else if (!this.stopped && cacheWriteBehavior !== CacheWriteBehavior.FORBID) {
if (shouldWriteResult(result, options.errorPolicy)) {
// Using a transaction here so we have a chance to read the result
// back from the cache before the watch callback fires as a result
Expand All @@ -330,6 +336,7 @@ export class QueryInfo {
query: this.document!,
data: result.data as T,
variables: options.variables,
overwrite: cacheWriteBehavior === CacheWriteBehavior.OVERWRITE,
});

this.lastWrite = {
Expand Down
36 changes: 23 additions & 13 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ import {
} from './types';
import { LocalState } from './LocalState';

import { QueryInfo, QueryStoreValue, shouldWriteResult } from './QueryInfo';
import {
QueryInfo,
QueryStoreValue,
shouldWriteResult,
CacheWriteBehavior,
} from './QueryInfo';

const { hasOwnProperty } = Object.prototype;

Expand Down Expand Up @@ -822,7 +827,7 @@ export class QueryManager<TStore> {

private getResultsFromLink<TData, TVars>(
queryInfo: QueryInfo,
allowCacheWrite: boolean,
cacheWriteBehavior: CacheWriteBehavior,
options: Pick<WatchQueryOptions<TVars, TData>,
| "variables"
| "context"
Expand All @@ -848,7 +853,7 @@ export class QueryManager<TStore> {
graphQLErrors: result.errors,
}));
}
queryInfo.markResult(result, options, allowCacheWrite);
queryInfo.markResult(result, options, cacheWriteBehavior);
queryInfo.markReady();
}

Expand Down Expand Up @@ -1076,8 +1081,13 @@ export class QueryManager<TStore> {
return fromData(data);
};

const resultsFromLink = (allowCacheWrite: boolean) =>
this.getResultsFromLink<TData, TVars>(queryInfo, allowCacheWrite, {
const cacheWriteBehavior =
fetchPolicy === "no-cache" ? CacheWriteBehavior.FORBID :
networkStatus === NetworkStatus.refetch ? CacheWriteBehavior.OVERWRITE :
CacheWriteBehavior.MERGE;

const resultsFromLink = () =>
this.getResultsFromLink<TData, TVars>(queryInfo, cacheWriteBehavior, {
variables,
context,
fetchPolicy,
Expand All @@ -1103,12 +1113,12 @@ export class QueryManager<TStore> {
if (returnPartialData || shouldNotify) {
return [
resultsFromCache(diff),
resultsFromLink(true),
resultsFromLink(),
];
}

return [
resultsFromLink(true),
resultsFromLink(),
];
}

Expand All @@ -1118,12 +1128,12 @@ export class QueryManager<TStore> {
if (diff.complete || returnPartialData || shouldNotify) {
return [
resultsFromCache(diff),
resultsFromLink(true),
resultsFromLink(),
];
}

return [
resultsFromLink(true),
resultsFromLink(),
];
}

Expand All @@ -1136,11 +1146,11 @@ export class QueryManager<TStore> {
if (shouldNotify) {
return [
resultsFromCache(readCache()),
resultsFromLink(true),
resultsFromLink(),
];
}

return [resultsFromLink(true)];
return [resultsFromLink()];

case "no-cache":
if (shouldNotify) {
Expand All @@ -1149,11 +1159,11 @@ export class QueryManager<TStore> {
// cache.diff, but instead returns a { complete: false } stub result
// when there is no queryInfo.diff already defined.
resultsFromCache(queryInfo.getDiff()),
resultsFromLink(false),
resultsFromLink(),
];
}

return [resultsFromLink(false)];
return [resultsFromLink()];

case "standby":
return [];
Expand Down

0 comments on commit a16205e

Please sign in to comment.