From 07d89c4538fa563138dd99b2135376e0e2eab736 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 10 Jun 2020 16:27:07 -0400 Subject: [PATCH] Replace options.isReference(ref, true) with options.canRead(ref). (#6425) After thinking more about PR #6413, which has been merged but not yet released, I think it makes more sense to have a separate helper function for read, merge, and modify functions that determines if a given object can be read with options.readField. It was tempting to call this helper function options.isReadable, but options.canRead is shorter and less easily confused with options.isReference (which matters for auto-completion and dyslexic programmers), so options.canRead is what I settled on. This new helper makes it easy to filter out dangling references from lists of data correctly, without having to know if the list elements are normalized Reference objects, or non-normalized StoreObjects: new InMemoryCache({ typePolicies: { Person: { fields: { friends(existing, { canRead }) { return existing ? existing.filter(canRead) : []; }, }, }, }, }) Using isReference(friend, true) here would have worked only if isReference(friend) was true, whereas canRead(friend) also returns true when friend is a non-Reference object with readable fields, which can happen if the Friend type has no ID and/or keyFields:false. Now that we have options.canRead and options.readField, I think the use cases for options.isReference are probably pretty scarce, but I don't want to yank isReference away from folks who have been using it in the betas/RCs. Please let us know if you have a specific use case for options.isReference that is not covered by options.canRead, so we can decide how much documentation to give the various helpers. --- src/cache/core/types/common.ts | 9 ++++--- src/cache/inmemory/__tests__/readFromStore.ts | 16 +++++-------- src/cache/inmemory/entityStore.ts | 24 ++++++++----------- src/cache/inmemory/policies.ts | 14 +++++++---- src/cache/inmemory/readFromStore.ts | 2 +- src/cache/inmemory/types.ts | 4 ++-- src/cache/inmemory/writeToStore.ts | 2 +- 7 files changed, 34 insertions(+), 37 deletions(-) diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts index 4f1681a853a..874bfaae4f6 100644 --- a/src/cache/core/types/common.ts +++ b/src/cache/core/types/common.ts @@ -4,6 +4,7 @@ import { Reference, StoreObject, StoreValue, + isReference, } from '../../../core'; // The Readonly type only really works for object types, since it marks @@ -49,17 +50,15 @@ export type ToReferenceFunction = ( mergeIntoStore?: boolean, ) => Reference | undefined; -export type IsReferenceFunction = ( - candidate: any, - mustBeValid?: boolean, -) => candidate is Reference; +export type CanReadFunction = (value: StoreValue) => boolean; export type Modifier = (value: T, details: { DELETE: any; fieldName: string; storeFieldName: string; readField: ReadFieldFunction; - isReference: IsReferenceFunction; + canRead: CanReadFunction; + isReference: typeof isReference; toReference: ToReferenceFunction; }) => T; diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts index 8960e8c4478..c91eee256d1 100644 --- a/src/cache/inmemory/__tests__/readFromStore.ts +++ b/src/cache/inmemory/__tests__/readFromStore.ts @@ -923,23 +923,19 @@ describe('reading from the store', () => { Deity: { keyFields: ["name"], fields: { - 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) - ) : []; + children(offspring: Reference[], { canRead }) { + // Automatically filter out any dangling references, and + // supply a default empty array if !offspring. + return offspring ? offspring.filter(canRead) : []; }, }, }, Query: { fields: { - ruler(ruler, { isReference, toReference }) { + ruler(ruler, { canRead, toReference }) { // If the throne is empty, promote Apollo! - return isReference(ruler, true) ? ruler : toReference({ + return canRead(ruler) ? ruler : toReference({ __typename: "Deity", name: "Apollo", }); diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index c1de35815fb..b21639845a8 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -21,8 +21,8 @@ import { Modifiers, ReadFieldFunction, ReadFieldOptions, - IsReferenceFunction, ToReferenceFunction, + CanReadFunction, } from '../core/types/common'; const DELETE: any = Object.create(null); @@ -136,7 +136,7 @@ export abstract class EntityStore implements NormalizedCache { from: from || makeReference(dataId), } : fieldNameOrOptions, { - isReference: this.isReference, + canRead: this.canRead, toReference: this.toReference, getFieldValue: this.getFieldValue, }, @@ -157,6 +157,7 @@ export abstract class EntityStore implements NormalizedCache { storeFieldName, isReference, toReference: this.toReference, + canRead: this.canRead, readField, }); if (newValue === DELETE) newValue = void 0; @@ -351,18 +352,13 @@ export abstract class EntityStore implements NormalizedCache { : objectOrReference && objectOrReference[storeFieldName] ) as SafeReadonly; - // 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)); + // Returns true for non-normalized StoreObjects and non-dangling + // References, indicating that readField(name, objOrRef) has a chance of + // working. Useful for filtering out dangling references from lists. + public canRead: CanReadFunction = objOrRef => { + return isReference(objOrRef) + ? this.has(objOrRef.__ref) + : typeof objOrRef === "object"; }; // Bound function that converts an object with a __typename and primary diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index 42b87d1d1aa..dd7e29f2ed2 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -37,10 +37,10 @@ import { InMemoryCache } from './inMemoryCache'; import { SafeReadonly, FieldSpecifier, - IsReferenceFunction, ToReferenceFunction, ReadFieldFunction, ReadFieldOptions, + CanReadFunction, } from '../core/types/common'; export type TypePolicies = { @@ -133,7 +133,7 @@ export interface FieldFunctionOptions< variables?: TVars; // Utilities for dealing with { __ref } objects. - isReference: IsReferenceFunction; + isReference: typeof isReference; toReference: ToReferenceFunction; // A handy place to put field-specific data that you want to survive @@ -154,6 +154,11 @@ export interface FieldFunctionOptions< // immutable data (enforced with Object.freeze in development). readField: ReadFieldFunction; + // Returns true for non-normalized StoreObjects and non-dangling + // References, indicating that readField(name, objOrRef) has a chance of + // working. Useful for filtering out dangling references from lists. + canRead: CanReadFunction; + // Instead of just merging objects with { ...existing, ...incoming }, this // helper function can be used to merge objects in a way that respects any // custom merge functions defined for their fields. @@ -652,7 +657,7 @@ export class Policies { export interface ReadMergeContext { variables?: Record; - isReference: IsReferenceFunction; + canRead: CanReadFunction; toReference: ToReferenceFunction; getFieldValue: FieldValueGetter; } @@ -673,10 +678,11 @@ function makeFieldFunctionOptions( fieldName, storeFieldName, variables, - isReference: context.isReference, + isReference, toReference: context.toReference, storage, cache: policies.cache, + canRead: context.canRead, readField( fieldNameOrOptions: string | ReadFieldOptions, diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index f583583bbda..4143f72d5d3 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -142,8 +142,8 @@ export class StoreReader { variables, varString: JSON.stringify(variables), fragmentMap: createFragmentMap(getFragmentDefinitions(query)), - isReference: store.isReference, toReference: store.toReference, + canRead: store.canRead, getFieldValue: store.getFieldValue, path: [], }, diff --git a/src/cache/inmemory/types.ts b/src/cache/inmemory/types.ts index 13513bb9d12..6f2d83466e4 100644 --- a/src/cache/inmemory/types.ts +++ b/src/cache/inmemory/types.ts @@ -12,7 +12,7 @@ import { Modifier, Modifiers, ToReferenceFunction, - IsReferenceFunction, + CanReadFunction, } from '../core/types/common'; export { StoreObject, StoreValue, Reference } @@ -60,8 +60,8 @@ export interface NormalizedCache { release(rootId: string): number; getFieldValue: FieldValueGetter; - isReference: IsReferenceFunction; toReference: ToReferenceFunction; + canRead: CanReadFunction; } /** diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index 85e2e21a713..ee32ef6daa5 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -112,8 +112,8 @@ export class StoreWriter { variables, varString: JSON.stringify(variables), fragmentMap: createFragmentMap(getFragmentDefinitions(query)), - isReference: store.isReference, toReference: store.toReference, + canRead: store.canRead, getFieldValue: store.getFieldValue, }, });