Skip to content

Commit

Permalink
Ensure cache.broadcastWatch passes all relevant WatchOptions to `…
Browse files Browse the repository at this point in the history
…cache.diff` (#8832)

Fixes #8831.
  • Loading branch information
benjamn authored Sep 27, 2021
1 parent 90ac2b5 commit b7f89f3
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
- Prevent optimistic cache evictions from evicting non-optimistic data. <br/>
[@benjamn](https://github.com/benjamn) in [#8829](https://github.com/apollographql/apollo-client/pull/8829)

- Ensure `cache.broadcastWatch` passes all relevant `WatchOptions` to `cache.diff` as `DiffOptions`. <br/>
[@benjamn](https://github.com/benjamn) in [#8832](https://github.com/apollographql/apollo-client/pull/8832)

## Apollo Client 3.4.13

### Bug Fixes
Expand Down
164 changes: 164 additions & 0 deletions src/cache/inmemory/__tests__/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { wrap } from 'optimism';
import { StoreReader } from '../readFromStore';
import { StoreWriter } from '../writeToStore';
import { ObjectCanon } from '../object-canon';
import { TypePolicies } from '../policies';

disableFragmentWarnings();

Expand Down Expand Up @@ -1911,6 +1912,169 @@ describe("InMemoryCache#broadcastWatches", function () {
received2AllCaps,
]);
});

it("should pass WatchOptions through to cache.diff", () => {
const typePolicies: TypePolicies = {
Query: {
fields: {
object(_, { variables }) {
return { name: variables?.name ?? "UNKNOWN" };
},
},
},
};

const canonicalCache = new InMemoryCache({
canonizeResults: true,
typePolicies,
});

const nonCanonicalCache = new InMemoryCache({
canonizeResults: false,
typePolicies,
});

const query = gql`
query {
object {
name
}
}
`;

const unwatchers = new Set<() => void>();

type Diff = Cache.DiffResult<{
object: {
name: string;
};
}>;
const diffs: Record<string, Diff[]> = Object.create(null);
function addDiff(name: string, diff: Diff) {
(diffs[name] || (diffs[name] = [])).push(diff);
}

const commonWatchOptions = {
query,
optimistic: true,
immediate: true,
callback(diff: Diff) {
addDiff(diff.result!.object.name, diff);
},
};

unwatchers.add(canonicalCache.watch({
...commonWatchOptions,
variables: { name: "canonicalByDefault" },
// Pass nothing for canonizeResults to let the default for canonicalCache
// (true) prevail.
}));

unwatchers.add(nonCanonicalCache.watch({
...commonWatchOptions,
variables: { name: "nonCanonicalByDefault" },
// Pass nothing for canonizeResults to let the default for
// nonCanonicalCache (false) prevail.
}));

unwatchers.add(nonCanonicalCache.watch({
...commonWatchOptions,
variables: { name: "canonicalByChoice" },
canonizeResults: true, // Override the default.
}));

unwatchers.add(canonicalCache.watch({
...commonWatchOptions,
variables: { name: "nonCanonicalByChoice" },
canonizeResults: false, // Override the default.
}));

function makeDiff(name: string): Diff {
return {
complete: true,
result: {
object: { name },
},
};
}

const canonicalByDefaultDiff = makeDiff("canonicalByDefault");
const nonCanonicalByDefaultDiff = makeDiff("nonCanonicalByDefault");
const canonicalByChoiceDiff = makeDiff("canonicalByChoice");
const nonCanonicalByChoiceDiff = makeDiff("nonCanonicalByChoice");

expect(diffs).toEqual({
canonicalByDefault: [canonicalByDefaultDiff],
nonCanonicalByDefault: [nonCanonicalByDefaultDiff],
canonicalByChoice: [canonicalByChoiceDiff],
nonCanonicalByChoice: [nonCanonicalByChoiceDiff],
});

[ canonicalCache,
nonCanonicalCache,
].forEach(cache => {
// Hack: delete every watch.lastDiff, so subsequent results will be
// broadcast, even though they are deeply equal to the previous results.
cache["watches"].forEach(watch => {
delete watch.lastDiff;
});
});

// Evict Query.object to invalidate the result cache.
canonicalCache.evict({
fieldName: "object",
});
nonCanonicalCache.evict({
fieldName: "object",
});

// Every watcher receives the same (deeply equal) Diff a second time.
expect(diffs).toEqual({
canonicalByDefault: [
canonicalByDefaultDiff,
canonicalByDefaultDiff,
],
nonCanonicalByDefault: [
nonCanonicalByDefaultDiff,
nonCanonicalByDefaultDiff,
],
canonicalByChoice: [
canonicalByChoiceDiff,
canonicalByChoiceDiff,
],
nonCanonicalByChoice: [
nonCanonicalByChoiceDiff,
nonCanonicalByChoiceDiff,
],
});

function expectCanonical(name: string) {
const count = diffs[name].length;
const firstDiff = diffs[name][0];
for (let i = 1; i < count; ++i) {
expect(firstDiff).toEqual(diffs[name][i]);
expect(firstDiff.result).toBe(diffs[name][i].result);
}
}

function expectNonCanonical(name: string) {
const count = diffs[name].length;
const firstDiff = diffs[name][0];
for (let i = 1; i < count; ++i) {
expect(firstDiff).toEqual(diffs[name][i]);
expect(firstDiff.result).not.toBe(diffs[name][i].result);
}
}

// However, some of the diff.result objects are canonized and thus ===, and
// others are deeply equal but not canonized (and thus not ===).
expectCanonical("canonicalByDefault");
expectCanonical("canonicalByChoice");
expectNonCanonical("nonCanonicalByDefault");
expectNonCanonical("nonCanonicalByChoice");

unwatchers.forEach(unwatch => unwatch());
});
});

describe("InMemoryCache#modify", () => {
Expand Down
13 changes: 8 additions & 5 deletions src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -539,11 +539,14 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
options?: BroadcastOptions,
) {
const { lastDiff } = c;
const diff = this.diff<any>({
query: c.query,
variables: c.variables,
optimistic: c.optimistic,
});

// Both WatchOptions and DiffOptions extend ReadOptions, and DiffOptions
// currently requires no additional properties, so we can use c (a
// WatchOptions object) as DiffOptions, without having to allocate a new
// object, and without having to enumerate the relevant properties (query,
// variables, etc.) explicitly. There will be some additional properties
// (lastDiff, callback, etc.), but cache.diff ignores them.
const diff = this.diff<any>(c);

if (options) {
if (c.optimistic &&
Expand Down

0 comments on commit b7f89f3

Please sign in to comment.