Skip to content

Commit

Permalink
Replace options.isReference(ref, true) with options.canRead(ref). (#6425
Browse files Browse the repository at this point in the history
)

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.
  • Loading branch information
benjamn committed Jun 10, 2020
1 parent 9450938 commit 07d89c4
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 37 deletions.
9 changes: 4 additions & 5 deletions src/cache/core/types/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
Reference,
StoreObject,
StoreValue,
isReference,
} from '../../../core';

// The Readonly<T> type only really works for object types, since it marks
Expand Down Expand Up @@ -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<T> = (value: T, details: {
DELETE: any;
fieldName: string;
storeFieldName: string;
readField: ReadFieldFunction;
isReference: IsReferenceFunction;
canRead: CanReadFunction;
isReference: typeof isReference;
toReference: ToReferenceFunction;
}) => T;

Expand Down
16 changes: 6 additions & 10 deletions src/cache/inmemory/__tests__/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
});
Expand Down
24 changes: 10 additions & 14 deletions src/cache/inmemory/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import {
Modifiers,
ReadFieldFunction,
ReadFieldOptions,
IsReferenceFunction,
ToReferenceFunction,
CanReadFunction,
} from '../core/types/common';

const DELETE: any = Object.create(null);
Expand Down Expand Up @@ -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,
},
Expand All @@ -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;
Expand Down Expand Up @@ -351,18 +352,13 @@ 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));
// 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
Expand Down
14 changes: 10 additions & 4 deletions src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ import { InMemoryCache } from './inMemoryCache';
import {
SafeReadonly,
FieldSpecifier,
IsReferenceFunction,
ToReferenceFunction,
ReadFieldFunction,
ReadFieldOptions,
CanReadFunction,
} from '../core/types/common';

export type TypePolicies = {
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -652,7 +657,7 @@ export class Policies {

export interface ReadMergeContext {
variables?: Record<string, any>;
isReference: IsReferenceFunction;
canRead: CanReadFunction;
toReference: ToReferenceFunction;
getFieldValue: FieldValueGetter;
}
Expand All @@ -673,10 +678,11 @@ function makeFieldFunctionOptions(
fieldName,
storeFieldName,
variables,
isReference: context.isReference,
isReference,
toReference: context.toReference,
storage,
cache: policies.cache,
canRead: context.canRead,

readField<T>(
fieldNameOrOptions: string | ReadFieldOptions,
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
},
Expand Down
4 changes: 2 additions & 2 deletions src/cache/inmemory/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
Modifier,
Modifiers,
ToReferenceFunction,
IsReferenceFunction,
CanReadFunction,
} from '../core/types/common';
export { StoreObject, StoreValue, Reference }

Expand Down Expand Up @@ -60,8 +60,8 @@ export interface NormalizedCache {
release(rootId: string): number;

getFieldValue: FieldValueGetter;
isReference: IsReferenceFunction;
toReference: ToReferenceFunction;
canRead: CanReadFunction;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
});
Expand Down

0 comments on commit 07d89c4

Please sign in to comment.