Reactivate reactive variables when InMemoryCache acquires first watch…

…er (#7657)

CHANGELOG.md

@@ -5,6 +5,9 @@
 - Catch updates in `useReactiveVar` with an additional check. <br/>
   [@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. <br/>
+  [@benjamn](https://github.com/benjamn) in [#7657](https://github.com/apollographql/apollo-client/pull/7657)
+
 ## Apollo Client 3.3.7
 
 ### Bug Fixes

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<any>[] = [];
+    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({

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<NormalizedCacheObject> {
   }
 
   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);

src/cache/inmemory/reactiveVars.ts

@@ -35,10 +35,20 @@ const varsByCache = new WeakMap<ApolloCache<any>, Set<ReactiveVar<any>>>();
 
 export function forgetCache(cache: ApolloCache<any>) {
   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<any>) {
+  const vars = varsByCache.get(cache);
+  if (vars) vars.forEach(rv => rv.attachCache(cache));
 }
 
 export function makeVar<T>(value: T): ReactiveVar<T> {
@@ -86,14 +96,7 @@ export function makeVar<T>(value: T): ReactiveVar<T> {
     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;
 }