Skip to content

Commit

Permalink
Add Cache.WatchOptions["lastDiff"] deduplication to broadcastWatch.
Browse files Browse the repository at this point in the history
The goal of broadcastWatches is to notify cache watchers of any new data
resulting from cache writes.

However, it's possible for cache writes to invalidate watched queries in a
way that does not result in any differences in the resulting data, so this
watch.lastDiff caching saves us from triggering a redundant broadcast of
exactly the same data again.

Note: thanks to #7439, when two result objects are deeply equal to each
another, they will automatically also be === to each other, which is what
allows us to get away with the !== check in this code.
  • Loading branch information
benjamn committed Mar 23, 2021
1 parent 4be203e commit 532de44
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 35 deletions.
1 change: 1 addition & 0 deletions src/cache/core/types/Cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export namespace Cache {
export interface WatchOptions extends ReadOptions {
immediate?: boolean;
callback: WatchCallback;
lastDiff?: DiffResult<any>;
}

export interface EvictOptions {
Expand Down
27 changes: 5 additions & 22 deletions src/cache/inmemory/__tests__/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1492,22 +1492,11 @@ describe('Cache', () => {
expect(dirtied.has(abInfo.watch)).toBe(true);
expect(dirtied.has(bInfo.watch)).toBe(false);

expect(last(aInfo.diffs)).toEqual({
complete: true,
result: {
a: "ay",
},
});

expect(last(abInfo.diffs)).toEqual({
complete: true,
result: {
a: "ay",
b: "bee",
},
});

expect(bInfo.diffs.length).toBe(0);
// No new diffs should have been generated, since we only invalidated
// fields using cache.modify, and did not change any field values.
expect(aInfo.diffs).toEqual([]);
expect(abInfo.diffs).toEqual([]);
expect(bInfo.diffs).toEqual([]);

aInfo.cancel();
abInfo.cancel();
Expand Down Expand Up @@ -1686,7 +1675,6 @@ describe("InMemoryCache#broadcastWatches", function () {
expect(receivedCallbackResults).toEqual([
received1,
// New results:
received1,
received2,
]);

Expand Down Expand Up @@ -1714,7 +1702,6 @@ describe("InMemoryCache#broadcastWatches", function () {
}];

expect(receivedCallbackResults).toEqual([
received1,
received1,
received2,
// New results:
Expand All @@ -1734,16 +1721,12 @@ describe("InMemoryCache#broadcastWatches", function () {
}];

expect(receivedCallbackResults).toEqual([
received1,
received1,
received2,
received3,
received4,
// New results:
received1,
received2AllCaps,
received3,
received4,
]);
});
});
Expand Down
23 changes: 11 additions & 12 deletions src/cache/inmemory/__tests__/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1419,10 +1419,10 @@ describe('reading from the store', () => {

const diffs: Cache.DiffResult<any>[] = [];

function watch() {
function watch(immediate = true) {
return cache.watch({
query: rulerQuery,
immediate: true,
immediate,
optimistic: true,
callback(diff) {
diffs.push(diff);
Expand Down Expand Up @@ -1764,22 +1764,17 @@ describe('reading from the store', () => {
diffWithZeusAsRuler,
]);

// Rewatch the rulerQuery, which will populate the same diffs array
// that we were using before.
const cancel2 = watch();

const diffWithApolloAsRuler = {
complete: true,
result: apolloRulerResult,
};
// Rewatch the rulerQuery, but avoid delivering an immediate initial
// result (by passing false), so that we can use cache.modify to
// trigger the delivery of diffWithApolloAsRuler below.
const cancel2 = watch(false);

expect(diffs).toEqual([
initialDiff,
diffWithoutSon1,
diffWithoutDevouredSons,
diffWithChildrenOfZeus,
diffWithZeusAsRuler,
diffWithApolloAsRuler,
]);

cache.modify({
Expand All @@ -1797,6 +1792,11 @@ describe('reading from the store', () => {

cancel2();

const diffWithApolloAsRuler = {
complete: true,
result: apolloRulerResult,
};

// The cache.modify call should have triggered another diff, since we
// overwrote the ROOT_QUERY.ruler field with a valid Reference to the
// Apollo entity object.
Expand All @@ -1807,7 +1807,6 @@ describe('reading from the store', () => {
diffWithChildrenOfZeus,
diffWithZeusAsRuler,
diffWithApolloAsRuler,
diffWithApolloAsRuler,
]);

expect(
Expand Down
4 changes: 3 additions & 1 deletion src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,8 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
}
}

c.callback(diff);
if (!c.lastDiff || c.lastDiff.result !== diff.result) {
c.callback(c.lastDiff = diff);
}
}
}

0 comments on commit 532de44

Please sign in to comment.