Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reactivate reactive variables when InMemoryCache acquires first watcher. #7657

Merged
merged 3 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
98 changes: 98 additions & 0 deletions src/cache/inmemory/__tests__/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
15 changes: 14 additions & 1 deletion src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
27 changes: 15 additions & 12 deletions src/cache/inmemory/reactiveVars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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;
}
Expand Down