Skip to content

Commit

Permalink
Include watch.callback in maybeBroadcastWatch cache keys.
Browse files Browse the repository at this point in the history
Fixes #5733, which was caused by multiple watches having the same cache
key, so none except the first were ever re-broadcast, since the first
broadcast had the side effect of marking the later watches as clean,
before this.watches.forEach had a chance to visit them.

Background: PR #5644 reenabled an important optimization for the
InMemoryCache#broadcastWatches method, allowing it to skip watches whose
results have not changed. Unfortunately, while this optimization correctly
determined whether the result had changed, it did not account for the
possibility of multiple distinct consumers of the same result, which can
happen (for example) when multiple components use the same query and
variables via different useQuery calls.

Fortunately, the fix is straightforward (if not exactly obvious): in order
to assign distinct consumers different cache keys, it suffices to include
the provided watch.callback function in the cache key.

A more drastic way to fix #5733 would be to remove the caching of
maybeBroadcastWatch entirely, which would not be a huge loss because the
underlying cache.diff method is also cached. For now, though, with that
backup option in mind, I'd like to preserve this optimization unless it
causes any further problems.
  • Loading branch information
benjamn committed Jan 3, 2020
1 parent 1b9a392 commit 74b65fe
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
const { optimistic, rootId, variables } = c;
return store.makeCacheKey(
c.query,
// Different watches can have the same query, optimistic
// status, rootId, and variables, but if their callbacks are
// different, the (identical) result needs to be delivered to
// each distinct callback. The easiest way to achieve that
// separation is to include c.callback in the cache key for
// maybeBroadcastWatch calls. See issue #5733.
c.callback,
JSON.stringify({ optimistic, rootId, variables }),
);
}
Expand Down

0 comments on commit 74b65fe

Please sign in to comment.