Skip to content

Commit

Permalink
Make makeVar a global function instead of a method of InMemoryCache.
Browse files Browse the repository at this point in the history
The makeVar method was originally attached to InMemoryCache so that we
could call cache.broadcastWatches() whenever the variable was updated.
See #5799 and #5976 for background.

However, as a number of developers have reported, requiring access to an
InMemoryCache to create a ReactiveVar can be awkward, since the code that
calls makeVar may not be colocated with the code that creates the cache,
and it is often desirable to create and initialize reactive variables
before the cache has been created.

As this commit shows, the ReactiveVar function can infer the current
InMemoryCache from a contextual Slot, when called without arguments (that
is, when reading the variable). When the variable is updated (by passing a
new value to the ReactiveVar function), any caches that previously read
the variable will be notified of the update. Since this logic happens at
variable access time rather than variable creation time, makeVar can be a
free-floating global function, importable directly from @apollo/client.

This new system allows the variable to become associated with any number
of InMemoryCache instances, whereas previously a given variable was only
ever associated with one InMemoryCache. Note: when I say "any number" I
very much mean to include zero, since a ReactiveVar that has not been
associated with any caches yet can still be used as a container, and will
not trigger any broadcasts when updated.

The Slot class that makes this all work may seem like magic, but we have
been using it ever since Apollo Client 2.5 (#3394, via the optimism
library), so it has been amply battle-tested. This magic works.
  • Loading branch information
benjamn committed Jun 30, 2020
1 parent fe908c8 commit 651357f
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 55 deletions.
1 change: 1 addition & 0 deletions config/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const external = [
'graphql/language/visitor',
'graphql-tag',
'fast-json-stable-stringify',
'@wry/context',
'@wry/equality',
'react',
'zen-observable'
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
{
"name": "apollo-client",
"path": "./dist/apollo-client.cjs.min.js",
"maxSize": "24.25 kB"
"maxSize": "24.5 kB"
}
],
"peerDependencies": {
Expand Down
7 changes: 3 additions & 4 deletions src/__tests__/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { ApolloClient } from '..';
import subscribeAndCount from '../utilities/testing/subscribeAndCount';
import { itAsync } from '../utilities/testing/itAsync';
import { mockSingleLink } from '../utilities/testing/mocking/mockLink';
import { ObservableQuery, PossibleTypesMap } from '../core';
import { ObservableQuery, PossibleTypesMap, makeVar } from '../core';

describe('client', () => {
it('can be loaded via require', () => {
Expand Down Expand Up @@ -2815,6 +2815,8 @@ describe('@connection', () => {
});

itAsync('should broadcast changes for reactive variables', async (resolve, reject) => {
const aVar = makeVar(123);
const bVar = makeVar("asdf");
const cache: InMemoryCache = new InMemoryCache({
typePolicies: {
Query: {
Expand All @@ -2830,9 +2832,6 @@ describe('@connection', () => {
},
});

const aVar = cache.makeVar(123);
const bVar = cache.makeVar("asdf");

const client = new ApolloClient({ cache });

const obsQueries = new Set<ObservableQuery<any>>();
Expand Down
6 changes: 5 additions & 1 deletion src/cache/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ export {
export {
InMemoryCache,
InMemoryCacheConfig,
ReactiveVar,
} from './inmemory/inMemoryCache';

export {
ReactiveVar,
makeVar,
} from './inmemory/reactiveVars';

export {
defaultDataIdFromObject,
TypePolicies,
Expand Down
7 changes: 3 additions & 4 deletions src/cache/inmemory/__tests__/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import gql, { disableFragmentWarnings } from 'graphql-tag';

import { stripSymbols } from '../../../utilities/testing/stripSymbols';
import { cloneDeep } from '../../../utilities/common/cloneDeep';
import { makeReference, Reference } from '../../../core';
import { makeReference, Reference, makeVar } from '../../../core';
import { InMemoryCache, InMemoryCacheConfig } from '../inMemoryCache';

disableFragmentWarnings();
Expand Down Expand Up @@ -2393,8 +2393,9 @@ describe("InMemoryCache#modify", () => {
});
});

describe("cache.makeVar", () => {
describe("ReactiveVar and makeVar", () => {
function makeCacheAndVar(resultCaching: boolean) {
const nameVar = makeVar("Ben");
const cache: InMemoryCache = new InMemoryCache({
resultCaching,
typePolicies: {
Expand All @@ -2408,8 +2409,6 @@ describe("cache.makeVar", () => {
},
});

const nameVar = cache.makeVar("Ben");

const query = gql`
query {
onCall {
Expand Down
27 changes: 14 additions & 13 deletions src/cache/inmemory/__tests__/policies.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import gql from "graphql-tag";

import { InMemoryCache, ReactiveVar } from "../inMemoryCache";
import { InMemoryCache } from "../inMemoryCache";
import { ReactiveVar, makeVar } from "../reactiveVars";
import { Reference, StoreObject, ApolloClient, NetworkStatus } from "../../../core";
import { MissingFieldError } from "../..";
import { relayStylePagination } from "../../../utilities";
Expand Down Expand Up @@ -959,15 +960,15 @@ describe("type policies", function () {
result: {
read(_, { storage }) {
if (!storage!.jobName) {
storage!.jobName = cache.makeVar(undefined);
storage!.jobName = makeVar(undefined);
}
return storage!.jobName();
},
merge(_, incoming: string, { storage }) {
if (storage!.jobName) {
storage!.jobName(incoming);
} else {
storage!.jobName = cache.makeVar(incoming);
storage!.jobName = makeVar(incoming);
}
},
},
Expand Down Expand Up @@ -1417,6 +1418,16 @@ describe("type policies", function () {
});

it("readField helper function calls custom read functions", function () {
// Rather than writing ownTime data into the cache, we maintain it
// externally in this object:
const ownTimes: Record<string, ReactiveVar<number>> = {
"parent task": makeVar(2),
"child task 1": makeVar(3),
"child task 2": makeVar(4),
"grandchild task": makeVar(5),
"independent task": makeVar(11),
};

const cache = new InMemoryCache({
typePolicies: {
Agenda: {
Expand Down Expand Up @@ -1494,16 +1505,6 @@ describe("type policies", function () {
},
});

// Rather than writing ownTime data into the cache, we maintain it
// externally in this object:
const ownTimes: Record<string, ReactiveVar<number>> = {
"parent task": cache.makeVar(2),
"child task 1": cache.makeVar(3),
"child task 2": cache.makeVar(4),
"grandchild task": cache.makeVar(5),
"independent task": cache.makeVar(11),
};

cache.writeQuery({
query: gql`
query {
Expand Down
25 changes: 3 additions & 22 deletions src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import { StoreReader } from './readFromStore';
import { StoreWriter } from './writeToStore';
import { EntityStore, supportsResultCaching } from './entityStore';
import { makeVar } from './reactiveVars';
import {
defaultDataIdFromObject,
PossibleTypesMap,
Expand Down Expand Up @@ -53,6 +54,8 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
// cache.policies.addPossibletypes.
public readonly policies: Policies;

public readonly makeVar = makeVar;

constructor(config: InMemoryCacheConfig = {}) {
super();
this.config = { ...defaultConfig, ...config };
Expand Down Expand Up @@ -374,26 +377,4 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
optimistic: c.optimistic,
}));
}

private varDep = dep<ReactiveVar<any>>();

public makeVar<T>(value: T): ReactiveVar<T> {
const cache = this;
return function rv(newValue) {
if (arguments.length > 0) {
if (value !== newValue) {
value = newValue!;
cache.varDep.dirty(rv);
// In order to perform several ReactiveVar updates without
// broadcasting each time, use cache.performTransaction.
cache.broadcastWatches();
}
} else {
cache.varDep(rv);
}
return value;
};
}
}

export type ReactiveVar<T> = (newValue?: T) => T;
26 changes: 16 additions & 10 deletions src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
isFieldValueToBeMerged,
storeValueIsStoreObject,
} from './helpers';
import { cacheSlot } from './reactiveVars';
import { InMemoryCache } from './inMemoryCache';
import {
SafeReadonly,
Expand Down Expand Up @@ -525,20 +526,25 @@ export class Policies {
const read = policy && policy.read;

if (read) {
const storage = this.storageTrie.lookup(
isReference(objectOrReference)
? objectOrReference.__ref
: objectOrReference,
storeFieldName,
);

return read(existing, makeFieldFunctionOptions(
const readOptions = makeFieldFunctionOptions(
this,
objectOrReference,
options,
context,
storage,
)) as SafeReadonly<V>;
this.storageTrie.lookup(
isReference(objectOrReference)
? objectOrReference.__ref
: objectOrReference,
storeFieldName,
),
);

// Call read(existing, readOptions) with cacheSlot holding this.cache.
return cacheSlot.withValue(
this.cache,
read,
[existing, readOptions],
) as SafeReadonly<V>;
}

return existing;
Expand Down
46 changes: 46 additions & 0 deletions src/cache/inmemory/reactiveVars.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { Slot } from "@wry/context";
import { dep } from "optimism";
import { InMemoryCache } from "./inMemoryCache";

export type ReactiveVar<T> = (newValue?: T) => T;

const varDep = dep<ReactiveVar<any>>();

// Contextual Slot that acquires its value when custom read functions are
// called in Policies#readField.
export const cacheSlot = new Slot<InMemoryCache>();

export function makeVar<T>(value: T): ReactiveVar<T> {
const caches = new Set<InMemoryCache>();

return function rv(newValue) {
if (arguments.length > 0) {
if (value !== newValue) {
value = newValue!;
varDep.dirty(rv);
// Trigger broadcast for any caches that were previously involved
// in reading this variable.
caches.forEach(broadcast);
}
} else {
// When reading from the variable, obtain the current InMemoryCache
// from context via cacheSlot. This isn't entirely foolproof, but
// it's the same system that powers varDep.
const cache = cacheSlot.getValue();
if (cache) caches.add(cache);
varDep(rv);
}

return value;
};
}

type Broadcastable = InMemoryCache & {
// This method is protected in InMemoryCache, which we are ignoring, but
// we still want some semblance of type safety when we call it.
broadcastWatches: InMemoryCache["broadcastWatches"];
};

function broadcast(cache: Broadcastable) {
cache.broadcastWatches();
}

0 comments on commit 651357f

Please sign in to comment.