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

…6413) 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.
apollographql · Jun 9, 2020 · 6f8d1b7 · 6f8d1b7
1 parent 7f2b1b0
commit 6f8d1b7
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 29 deletions.
diff --git a/src/cache/core/types/common.ts b/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;
 

diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/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",
+              });
             },
           },
         },

diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts
@@ -21,6 +21,7 @@ import {
   Modifiers,
   ReadFieldFunction,
   ReadFieldOptions,
+  IsReferenceFunction,
   ToReferenceFunction,
 } from '../core/types/common';
 
@@ -135,6 +136,7 @@ export abstract class EntityStore implements NormalizedCache {
           from: from || makeReference(dataId),
         } : fieldNameOrOptions,
         {
+          isReference: this.isReference,
           toReference: this.toReference,
           getFieldValue: this.getFieldValue,
         },
@@ -225,7 +227,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;
   }
@@ -347,10 +351,27 @@ export abstract class EntityStore implements NormalizedCache {
       : objectOrReference && objectOrReference[storeFieldName]
   ) as SafeReadonly<T>;
 
+  // Useful for determining if an object 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) &&
+      // Note: this lookup will find IDs only in this layer and any layers
+      // underneath it, even though there might be additional layers on
+      // top of this layer, known only to the InMemoryCache.
+      (!mustBeValid || this.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) => {
+  public toReference: ToReferenceFunction = (
+    object,
+    mergeIntoStore,
+  ) => {
     const [id] = this.policies.identify(object);
     if (id) {
       const ref = makeReference(id);
@@ -359,7 +380,7 @@ export abstract class EntityStore implements NormalizedCache {
       }
       return ref;
     }
-  }
+  };
 }
 
 export type FieldValueGetter = EntityStore["getFieldValue"];

diff --git a/src/cache/inmemory/policies.ts b/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
@@ -651,6 +652,7 @@ export class Policies {
 
 export interface ReadMergeContext {
   variables?: Record<string, any>;
+  isReference: IsReferenceFunction;
   toReference: ToReferenceFunction;
   getFieldValue: FieldValueGetter;
 }
@@ -662,17 +664,17 @@ function makeFieldFunctionOptions(
   storage: StorageType | null,
   context: ReadMergeContext,
 ): FieldFunctionOptions {
-  const { toReference, getFieldValue, variables } = context;
   const storeFieldName = policies.getStoreFieldName(fieldSpec);
   const fieldName = fieldNameFromStoreName(storeFieldName);
+  const variables = fieldSpec.variables || context.variables;
   return {
     args: argsFromFieldSpecifier(fieldSpec),
     field: fieldSpec.field || null,
     fieldName,
     storeFieldName,
     variables,
-    isReference,
-    toReference,
+    isReference: context.isReference,
+    toReference: context.toReference,
     storage,
     cache: policies.cache,
 
@@ -684,12 +686,17 @@ function makeFieldFunctionOptions(
         typeof fieldNameOrOptions === "string" ? {
           fieldName: fieldNameOrOptions,
           from,
-        } : fieldNameOrOptions;
+        } : { ...fieldNameOrOptions };
 
-      return policies.readField<T>(options.from ? options : {
-        ...options,
-        from: objectOrReference,
-      }, context);
+      if (void 0 === options.from) {
+        options.from = objectOrReference;
+      }
+
+      if (void 0 === options.variables) {
+        options.variables = variables;
+      }
+
+      return policies.readField<T>(options, context);
     },
 
     mergeObjects(existing, incoming) {
@@ -703,8 +710,8 @@ function makeFieldFunctionOptions(
       // parameter types of options.mergeObjects.
       if (existing && typeof existing === "object" &&
           incoming && typeof incoming === "object") {
-        const eType = getFieldValue(existing, "__typename");
-        const iType = getFieldValue(incoming, "__typename");
+        const eType = context.getFieldValue(existing, "__typename");
+        const iType = context.getFieldValue(incoming, "__typename");
         const typesDiffer = eType && iType && eType !== iType;
 
         const applied = policies.applyMerges(

diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts
@@ -142,6 +142,7 @@ export class StoreReader {
         variables,
         varString: JSON.stringify(variables),
         fragmentMap: createFragmentMap(getFragmentDefinitions(query)),
+        isReference: store.isReference,
         toReference: store.toReference,
         getFieldValue: store.getFieldValue,
         path: [],

diff --git a/src/cache/inmemory/types.ts b/src/cache/inmemory/types.ts
@@ -8,7 +8,12 @@ import {
 } from '../../utilities/graphql/storeUtils';
 import { FieldValueGetter } from './entityStore';
 import { KeyFieldsFunction } from './policies';
-import { ToReferenceFunction, Modifier, Modifiers } from '../core/types/common';
+import {
+  Modifier,
+  Modifiers,
+  ToReferenceFunction,
+  IsReferenceFunction,
+} from '../core/types/common';
 export { StoreObject, StoreValue, Reference }
 
 export interface IdGetterObj extends Object {
@@ -55,6 +60,7 @@ export interface NormalizedCache {
   release(rootId: string): number;
 
   getFieldValue: FieldValueGetter;
+  isReference: IsReferenceFunction;
   toReference: ToReferenceFunction;
 }
 

diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts
@@ -112,6 +112,7 @@ export class StoreWriter {
         variables,
         varString: JSON.stringify(variables),
         fragmentMap: createFragmentMap(getFragmentDefinitions(query)),
+        isReference: store.isReference,
         toReference: store.toReference,
         getFieldValue: store.getFieldValue,
       },