Support options.isReference(ref, true) to check Reference validity.

The story we're telling in #6412 about using custom read functions to
filter out dangling references works best if there's an easy way to check
whether a Reference points to existing data in the cache. Although we
could have introduced a new options.isValidReference helper function, I
think it makes sense to let the existing options.isReference function
handle this use case as well.

I ended up refactoring how the toReference function gets created and
passed around as well, since I want toReference and isReference to remain
together as much as possible. I considered making isReference a property
of EntityStore (like toReference used to be), but that would not have
worked because the new isReference(ref, true) functionality needs access
to the topmost layer of the cache, which only InMemoryCache knows about.
Long story short, both isReference and toReference are now methods of
InMemoryCache, rather than EntityStore.

src/cache/core/types/common.ts

@@ -4,7 +4,6 @@ import {
   Reference,
   StoreObject,
   StoreValue,
-  isReference,
 } from '../../../core';
 
 // The Readonly<T> type only really works for object types, since it marks
@@ -50,12 +49,17 @@ export type ToReferenceFunction = (
   mergeIntoStore?: boolean,
 ) => Reference | undefined;
 
+export type IsReferenceFunction = (
+  candidate: any,
+  mustBeValid?: boolean,
+) => candidate is Reference;
+
 export type Modifier<T> = (value: T, details: {
   DELETE: any;
   fieldName: string;
   storeFieldName: string;
   readField: ReadFieldFunction;
-  isReference: typeof isReference;
+  isReference: IsReferenceFunction;
   toReference: ToReferenceFunction;
 }) => T;
 

src/cache/inmemory/__tests__/readFromStore.ts

@@ -923,24 +923,26 @@ describe('reading from the store', () => {
         Deity: {
           keyFields: ["name"],
           fields: {
-            children(offspring: Reference[], { readField }) {
-              return offspring ? offspring.filter(child => {
-                // TODO Improve this test? Maybe isReference(ref, true)?
-                return void 0 !== readField("__typename", child);
-              }) : [];
+            children(offspring: Reference[], { isReference }) {
+              return offspring ? offspring.filter(
+                // The true argument here makes isReference return true
+                // only if child is a Reference object that points to
+                // valid entity data in the EntityStore (that is, not a
+                // dangling reference).
+                child => isReference(child, true)
+              ) : [];
             },
           },
         },
 
         Query: {
           fields: {
-            ruler(ruler, { toReference, readField }) {
-              // TODO Improve this test? Maybe !isReference(ruler, true)?
-              if (!ruler || void 0 === readField("__typename", ruler)) {
-                // If there's no official ruler, promote Apollo!
-                return toReference({ __typename: "Deity", name: "Apollo" });
-              }
-              return ruler;
+            ruler(ruler, { isReference, toReference }) {
+              // If the throne is empty, promote Apollo!
+              return isReference(ruler, true) ? ruler : toReference({
+                __typename: "Deity",
+                name: "Apollo",
+              });
             },
           },
         },

src/cache/inmemory/entityStore.ts

@@ -21,7 +21,6 @@ import {
   Modifiers,
   ReadFieldFunction,
   ReadFieldOptions,
-  ToReferenceFunction,
 } from '../core/types/common';
 
 const DELETE: any = Object.create(null);
@@ -126,6 +125,8 @@ export abstract class EntityStore implements NormalizedCache {
       let needToMerge = false;
       let allDeleted = true;
 
+      const { cache } = this.policies;
+
       const readField: ReadFieldFunction = <V = StoreValue>(
         fieldNameOrOptions: string | ReadFieldOptions,
         from?: StoreObject | Reference,
@@ -134,10 +135,7 @@ export abstract class EntityStore implements NormalizedCache {
           fieldName: fieldNameOrOptions,
           from: from || makeReference(dataId),
         } : fieldNameOrOptions,
-        {
-          toReference: this.toReference,
-          getFieldValue: this.getFieldValue,
-        },
+        this.getFieldValue,
       );
 
       Object.keys(storeObject).forEach(storeFieldName => {
@@ -153,8 +151,8 @@ export abstract class EntityStore implements NormalizedCache {
               DELETE,
               fieldName,
               storeFieldName,
-              isReference,
-              toReference: this.toReference,
+              isReference: cache.isReference,
+              toReference: cache.toReference,
               readField,
             });
           if (newValue === DELETE) newValue = void 0;
@@ -225,7 +223,9 @@ export abstract class EntityStore implements NormalizedCache {
       // queries, even if no cache data was modified by the eviction,
       // because queries may depend on computed fields with custom read
       // functions, whose values are not stored in the EntityStore.
-      this.group.dirty(options.id, options.fieldName || "__exists");
+      if (options.fieldName || evicted) {
+        this.group.dirty(options.id, options.fieldName || "__exists");
+      }
     }
     return evicted;
   }
@@ -346,20 +346,6 @@ export abstract class EntityStore implements NormalizedCache {
       ? this.get(objectOrReference.__ref, storeFieldName)
       : objectOrReference && objectOrReference[storeFieldName]
   ) as SafeReadonly<T>;
-
-  // Bound function that converts an object with a __typename and primary
-  // key fields to a Reference object. Pass true for mergeIntoStore if you
-  // would also like this object to be persisted into the store.
-  public toReference: ToReferenceFunction = (object, mergeIntoStore) => {
-    const [id] = this.policies.identify(object);
-    if (id) {
-      const ref = makeReference(id);
-      if (mergeIntoStore) {
-        this.merge(id, object);
-      }
-      return ref;
-    }
-  }
 }
 
 export type FieldValueGetter = EntityStore["getFieldValue"];

src/cache/inmemory/inMemoryCache.ts

@@ -7,7 +7,7 @@ import { dep, wrap } from 'optimism';
 import { ApolloCache, Transaction } from '../core/cache';
 import { Cache } from '../core/types/Cache';
 import { addTypenameToDocument } from '../../utilities/graphql/transform';
-import { StoreObject, Reference }  from '../../utilities/graphql/storeUtils';
+import { StoreObject, Reference, isReference, makeReference }  from '../../utilities/graphql/storeUtils';
 import {
   ApolloReducerConfig,
   NormalizedCacheObject,
@@ -22,6 +22,7 @@ import {
   TypePolicies,
 } from './policies';
 import { hasOwn } from './helpers';
+import { ToReferenceFunction, IsReferenceFunction } from '../core/types/common';
 
 export interface InMemoryCacheConfig extends ApolloReducerConfig {
   resultCaching?: boolean;
@@ -210,6 +211,31 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
     return this.policies.identify(object)[0];
   }
 
+  // Useful for determining if an object found in the EntityStore is a
+  // Reference or not. Pass true for mustBeValid to make isReference fail
+  // (return false) if the Reference does not refer to anything.
+  public isReference: IsReferenceFunction = (
+    candidate,
+    mustBeValid,
+  ): candidate is Reference => {
+    return isReference(candidate) &&
+      Boolean(!mustBeValid || this.optimisticData.has(candidate.__ref));
+  };
+
+  // Bound function that converts an object with a __typename and primary
+  // key fields to a Reference object. Pass true for mergeIntoStore if you
+  // would also like this object to be persisted into the store.
+  public toReference: ToReferenceFunction = (object, mergeIntoStore) => {
+    const [id] = this.policies.identify(object);
+    if (id) {
+      const ref = makeReference(id);
+      if (mergeIntoStore) {
+        this.optimisticData.merge(id, object);
+      }
+      return ref;
+    }
+  };
+
   public evict(options: Cache.EvictOptions): boolean {
     if (!options.id) {
       if (hasOwn.call(options, "id")) {

src/cache/inmemory/policies.ts

@@ -37,6 +37,7 @@ import { InMemoryCache } from './inMemoryCache';
 import {
   SafeReadonly,
   FieldSpecifier,
+  IsReferenceFunction,
   ToReferenceFunction,
   ReadFieldFunction,
   ReadFieldOptions,
@@ -132,7 +133,7 @@ export interface FieldFunctionOptions<
   variables?: TVars;
 
   // Utilities for dealing with { __ref } objects.
-  isReference: typeof isReference;
+  isReference: IsReferenceFunction;
   toReference: ToReferenceFunction;
 
   // A handy place to put field-specific data that you want to survive
@@ -495,7 +496,7 @@ export class Policies {
 
   public readField<V = StoreValue>(
     options: ReadFieldOptions,
-    context: ReadMergeContext,
+    getFieldValue: FieldValueGetter,
   ): SafeReadonly<V> | undefined {
     const objectOrReference = options.from;
     if (!objectOrReference) return;
@@ -504,14 +505,14 @@ export class Policies {
     if (!nameOrField) return;
 
     if (options.typename === void 0) {
-      const typename = context.getFieldValue<string>(
+      const typename = getFieldValue<string>(
         objectOrReference, "__typename");
       if (typename) options.typename = typename;
     }
 
     const storeFieldName = this.getStoreFieldName(options);
     const fieldName = fieldNameFromStoreName(storeFieldName);
-    const existing = context.getFieldValue<V>(objectOrReference, storeFieldName);
+    const existing = getFieldValue<V>(objectOrReference, storeFieldName);
     const policy = this.getFieldPolicy(options.typename, fieldName, false);
     const read = policy && policy.read;
 
@@ -528,7 +529,8 @@ export class Policies {
         objectOrReference,
         options,
         storage,
-        context,
+        getFieldValue,
+        options.variables,
       )) as SafeReadonly<V>;
     }
 
@@ -546,7 +548,8 @@ export class Policies {
   public applyMerges<T extends StoreValue>(
     existing: T | Reference,
     incoming: T | FieldValueToBeMerged,
-    context: ReadMergeContext,
+    getFieldValue: FieldValueGetter,
+    variables: Record<string, any> | undefined,
     storageKeys?: [string | StoreObject, string],
   ): T {
     if (isFieldValueToBeMerged(incoming)) {
@@ -585,9 +588,10 @@ export class Policies {
         { typename: incoming.__typename,
           fieldName,
           field,
-          variables: context.variables },
+          variables },
         storage,
-        context,
+        getFieldValue,
+        variables,
       )) as T;
     }
 
@@ -602,7 +606,8 @@ export class Policies {
         // have custom read or merge functions.
         void 0,
         item,
-        context,
+        getFieldValue,
+        variables,
       )) as T;
     }
 
@@ -626,9 +631,10 @@ export class Policies {
       Object.keys(i).forEach(storeFieldName => {
         const incomingValue = i[storeFieldName];
         const appliedValue = this.applyMerges(
-          context.getFieldValue(e, storeFieldName),
+          getFieldValue(e, storeFieldName),
           incomingValue,
-          context,
+          getFieldValue,
+          variables,
           // Avoid enabling options.storage when firstStorageKey is falsy,
           // which implies no options.storage object has ever been created
           // for a read/merge function for this field.
@@ -651,7 +657,6 @@ export class Policies {
 
 export interface ReadMergeContext {
   variables?: Record<string, any>;
-  toReference: ToReferenceFunction;
   getFieldValue: FieldValueGetter;
 }
 
@@ -660,9 +665,9 @@ function makeFieldFunctionOptions(
   objectOrReference: StoreObject | Reference | undefined,
   fieldSpec: FieldSpecifier,
   storage: StorageType | null,
-  context: ReadMergeContext,
+  getFieldValue: FieldValueGetter,
+  variables: Record<string, any> | undefined,
 ): FieldFunctionOptions {
-  const { toReference, getFieldValue, variables } = context;
   const storeFieldName = policies.getStoreFieldName(fieldSpec);
   const fieldName = fieldNameFromStoreName(storeFieldName);
   return {
@@ -671,8 +676,8 @@ function makeFieldFunctionOptions(
     fieldName,
     storeFieldName,
     variables,
-    isReference,
-    toReference,
+    isReference: policies.cache.isReference,
+    toReference: policies.cache.toReference,
     storage,
     cache: policies.cache,
 
@@ -684,12 +689,17 @@ function makeFieldFunctionOptions(
         typeof fieldNameOrOptions === "string" ? {
           fieldName: fieldNameOrOptions,
           from,
-        } : fieldNameOrOptions;
+        } : { ...fieldNameOrOptions };
+
+      if (void 0 === options.from) {
+        options.from = objectOrReference;
+      }
+
+      if (void 0 === options.variables) {
+        options.variables = variables;
+      }
 
-      return policies.readField<T>(options.from ? options : {
-        ...options,
-        from: objectOrReference,
-      }, context);
+      return policies.readField<T>(options, getFieldValue);
     },
 
     mergeObjects(existing, incoming) {
@@ -710,7 +720,8 @@ function makeFieldFunctionOptions(
         const applied = policies.applyMerges(
           typesDiffer ? void 0 : existing,
           incoming,
-          context,
+          getFieldValue,
+          variables,
         );
 
         if (

src/cache/inmemory/readFromStore.ts

@@ -125,7 +125,8 @@ export class StoreReader {
     variables,
     returnPartialData = true,
   }: DiffQueryAgainstStoreOptions): Cache.DiffResult<T> {
-    const policies = this.config.cache.policies;
+    const { cache } = this.config;
+    const policies = cache.policies;
 
     variables = {
       ...getDefaultValues(getQueryDefinition(query)),
@@ -142,7 +143,6 @@ export class StoreReader {
         variables,
         varString: JSON.stringify(variables),
         fragmentMap: createFragmentMap(getFragmentDefinitions(query)),
-        toReference: store.toReference,
         getFieldValue: store.getFieldValue,
         path: [],
       },
@@ -263,7 +263,7 @@ export class StoreReader {
           field: selection,
           variables: context.variables,
           from: objectOrReference,
-        }, context);
+        }, store.getFieldValue);
 
         const resultName = resultKeyNameFromField(selection);
         context.path.push(resultName);

src/cache/inmemory/types.ts

@@ -8,7 +8,10 @@ import {
 } from '../../utilities/graphql/storeUtils';
 import { FieldValueGetter } from './entityStore';
 import { KeyFieldsFunction } from './policies';
-import { ToReferenceFunction, Modifier, Modifiers } from '../core/types/common';
+import {
+  Modifier,
+  Modifiers,
+} from '../core/types/common';
 export { StoreObject, StoreValue, Reference }
 
 export interface IdGetterObj extends Object {
@@ -55,7 +58,6 @@ export interface NormalizedCache {
   release(rootId: string): number;
 
   getFieldValue: FieldValueGetter;
-  toReference: ToReferenceFunction;
 }
 
 /**