From 23c77543f98e80d1b82baec1e91b92d5b59ac835 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 5 Feb 2021 09:53:49 -0500 Subject: [PATCH] Reactivate reactive variables when InMemoryCache acquires first watcher (#7657) --- CHANGELOG.md | 3 + src/cache/inmemory/__tests__/cache.ts | 98 +++++++++++++++++++++++++++ src/cache/inmemory/inMemoryCache.ts | 15 +++- src/cache/inmemory/reactiveVars.ts | 27 ++++---- 4 files changed, 130 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84aa51e3901..b664d0851c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ - Catch updates in `useReactiveVar` with an additional check.
[@jcreighton](https://github.com/jcreighton) in [#7652](https://github.com/apollographql/apollo-client/pull/7652) +- Reactivate forgotten reactive variables whenever `InMemoryCache` acquires its first watcher.
+ [@benjamn](https://github.com/benjamn) in [#7657](https://github.com/apollographql/apollo-client/pull/7657) + ## Apollo Client 3.3.7 ### Bug Fixes diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts index f58542e4449..6703d10e59d 100644 --- a/src/cache/inmemory/__tests__/cache.ts +++ b/src/cache/inmemory/__tests__/cache.ts @@ -2658,6 +2658,104 @@ describe("ReactiveVar and makeVar", () => { expect(spy).toBeCalledWith(cache); }); + it("should recall forgotten vars once cache has watches again", () => { + const { cache, nameVar, query } = makeCacheAndVar(false); + const spy = jest.spyOn(nameVar, "forgetCache"); + + const diffs: Cache.DiffResult[] = []; + const watch = (immediate = true) => cache.watch({ + query, + optimistic: true, + immediate, + callback(diff) { + diffs.push(diff); + }, + }); + + const unwatchers = [ + watch(), + watch(), + watch(), + ]; + + const names = () => diffs.map(diff => diff.result.onCall.name); + + expect(diffs.length).toBe(3); + expect(names()).toEqual([ + "Ben", + "Ben", + "Ben", + ]); + + expect(cache["watches"].size).toBe(3); + expect(spy).not.toBeCalled(); + + unwatchers.pop()!(); + expect(cache["watches"].size).toBe(2); + expect(spy).not.toBeCalled(); + + unwatchers.shift()!(); + expect(cache["watches"].size).toBe(1); + expect(spy).not.toBeCalled(); + + nameVar("Hugh"); + expect(names()).toEqual([ + "Ben", + "Ben", + "Ben", + "Hugh", + ]); + + unwatchers.pop()!(); + expect(cache["watches"].size).toBe(0); + expect(spy).toBeCalledTimes(1); + expect(spy).toBeCalledWith(cache); + + // This update is ignored because the cache no longer has any watchers. + nameVar("ignored"); + expect(names()).toEqual([ + "Ben", + "Ben", + "Ben", + "Hugh", + ]); + + // Call watch(false) to avoid immediate delivery of the "ignored" name. + unwatchers.push(watch(false)); + expect(cache["watches"].size).toBe(1); + expect(names()).toEqual([ + "Ben", + "Ben", + "Ben", + "Hugh", + ]); + + // This is the test that would fail if cache.watch did not call + // recallCache(cache) upon re-adding the first watcher. + nameVar("Jenn"); + expect(names()).toEqual([ + "Ben", + "Ben", + "Ben", + "Hugh", + "Jenn", + ]); + + unwatchers.forEach(cancel => cancel()); + expect(spy).toBeCalledTimes(2); + expect(spy).toBeCalledWith(cache); + + // Ignored again because all watchers have been cancelled. + nameVar("also ignored"); + expect(names()).toEqual([ + "Ben", + "Ben", + "Ben", + "Hugh", + "Jenn", + ]); + }); + it("should broadcast only once for multiple reads of same variable", () => { const nameVar = makeVar("Ben"); const cache = new InMemoryCache({ diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index f9ef73954d0..6bc9d859bb8 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -20,7 +20,7 @@ import { import { StoreReader } from './readFromStore'; import { StoreWriter } from './writeToStore'; import { EntityStore, supportsResultCaching } from './entityStore'; -import { makeVar, forgetCache } from './reactiveVars'; +import { makeVar, forgetCache, recallCache } from './reactiveVars'; import { defaultDataIdFromObject, PossibleTypesMap, @@ -194,6 +194,19 @@ export class InMemoryCache extends ApolloCache { } public watch(watch: Cache.WatchOptions): () => void { + if (!this.watches.size) { + // In case we previously called forgetCache(this) because + // this.watches became empty (see below), reattach this cache to any + // reactive variables on which it previously depended. It might seem + // paradoxical that we're able to recall something we supposedly + // forgot, but the point of calling forgetCache(this) is to silence + // useless broadcasts while this.watches is empty, and to allow the + // cache to be garbage collected. If, however, we manage to call + // recallCache(this) here, this cache object must not have been + // garbage collected yet, and should resume receiving updates from + // reactive variables, now that it has a watcher to notify. + recallCache(this); + } this.watches.add(watch); if (watch.immediate) { this.maybeBroadcastWatch(watch); diff --git a/src/cache/inmemory/reactiveVars.ts b/src/cache/inmemory/reactiveVars.ts index 409c8611837..75761741901 100644 --- a/src/cache/inmemory/reactiveVars.ts +++ b/src/cache/inmemory/reactiveVars.ts @@ -35,10 +35,20 @@ const varsByCache = new WeakMap, Set>>(); export function forgetCache(cache: ApolloCache) { const vars = varsByCache.get(cache); - if (vars) { - consumeAndIterate(vars, rv => rv.forgetCache(cache)); - varsByCache.delete(cache); - } + if (vars) vars.forEach(rv => rv.forgetCache(cache)); +} + +// Calling forgetCache(cache) serves to silence broadcasts and allows the +// cache to be garbage collected. However, the varsByCache WeakMap +// preserves the set of reactive variables that were previously associated +// with this cache, which makes it possible to "recall" the cache at a +// later time, by reattaching it to those variables. If the cache has been +// garbage collected in the meantime, because it is no longer reachable, +// you won't be able to call recallCache(cache), and the cache will +// automatically disappear from the varsByCache WeakMap. +export function recallCache(cache: ApolloCache) { + const vars = varsByCache.get(cache); + if (vars) vars.forEach(rv => rv.attachCache(cache)); } export function makeVar(value: T): ReactiveVar { @@ -86,14 +96,7 @@ export function makeVar(value: T): ReactiveVar { return rv; }; - rv.forgetCache = cache => { - const deleted = caches.delete(cache); - if (deleted) { - const vars = varsByCache.get(cache); - if (vars) vars.delete(rv); - } - return deleted; - }; + rv.forgetCache = cache => caches.delete(cache); return rv; }