Repository: apollographql/apollo-client

Problem statement:
Reactive vars stop broadcasting changes to cache when query became inactive and active again (e.g. via navigation)
**Intended outcome:**
Changes made to local state managed with reactive variables should trigger update on every active query that depends on that variable. No matter if that was previously deactivated.

**Actual outcome:**
If query was deactivated and activated again update notification is no longer triggered.

**How to reproduce the issue:**
[CodeSandbox](https://codesandbox.io/s/apollogqlstatereactivevars-3u1op?file=/src/Home.js)

1. Click button "Click to increment" several times - change notification is broadcasted and component is re-rendered with good counter.
2. navigate to "About" and go back to "Home"
3. Click button again several times - **no update**
4. If we now navigate again to "About" and back to "Home" all works good again and in fact we are back at step one.

Based on my research I see that when query is deactivated cache is detached from reactive variable (forgetCache) but when it is activated again cache is not attached again as it is done in initial render or after second navigation. 

P.S. That works good if we are using useReactiveVar hook.

**Versions**
reproduced with: 3.3.6, 3.3.7



Hints:
@WojciechKulpa Thanks for the reproduction. I'll take a look at this soon (this week).
Any updates on this? We are facing the same issue and are looking forward to a fix for that! :)
@benjamn Cool if I re-assign this to myself? There's another issue related to reactive vars (#7609) that I'm going to look into, so might as well look into this too.

Patch:
diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts
--- 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<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);
diff --git a/src/cache/inmemory/reactiveVars.ts b/src/cache/inmemory/reactiveVars.ts
--- a/src/cache/inmemory/reactiveVars.ts
+++ b/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;
 }


Test patch:
diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts
--- 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<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({