Unify Field{Read,Merge}Options as much as possible, for consistency.

Although it was tempting to make the read function feel like a GraphQL
resolver function (parentObject, args, context, info), that signature
would not have been appropriate for the merge function, and having to
remember two totally different signatures for reading and merging did not
seem ideal.

Now, the only difference between the signatures is that the merge function
takes incoming data, to be merged with the existing data (if any):

  read(existing, { args, parentObject, field, variables })
  merge(existing, incoming, { args, parentObject, field, variables })

It would have been nice to use named options for everything, including the
existing and incoming parameters, but it's important for those parameters
to be positional so that the developer can specify their types, without
also having to provide a new type for the entire options object.

src/cache/inmemory/__tests__/diffAgainstStore.ts

@@ -959,7 +959,7 @@ describe('diffing queries against the store', () => {
         typePolicies: {
           Query: {
             fields: {
-              person(rootQuery, args) {
+              person(_, { args, parentObject: rootQuery }) {
                 expect(typeof args.id).toBe('number');
                 const id = this.identify({ __typename: 'Person', id: args.id });
                 expect(id).toBe(`Person:${JSON.stringify({ id: args.id })}`);

src/cache/inmemory/__tests__/policies.ts

@@ -1,6 +1,7 @@
 import gql from "graphql-tag";
 import { InMemoryCache } from "../inMemoryCache";
 import { StoreValue } from "../../../utilities";
+import { FieldPolicy } from "../policies";
 
 describe("type policies", function () {
   const bookQuery = gql`
@@ -396,7 +397,7 @@ describe("type policies", function () {
           Person: {
             keyFields: ["firstName", "lastName"],
             fields: {
-              fullName(person) {
+              fullName(_, { parentObject: person }) {
                 return `${person.firstName} ${person.lastName}`;
               },
             },
@@ -534,23 +535,20 @@ describe("type policies", function () {
               todos: {
                 keyArgs: [],
 
-                read(person, args, { storeFieldValue }) {
-                  return (storeFieldValue as any[]).slice(
+                read(existing: any[], { args }) {
+                  return existing.slice(
                     args.offset,
                     args.offset + args.limit,
                   );
                 },
 
-                merge({
-                  args,
-                  existingValue = [],
-                  incomingValue,
-                }): StoreValue[] {
+                merge(existing: any[], incoming: any[], { args }) {
+                  const copy = existing ? existing.slice(0) : [];
                   const limit = args.offset + args.limit;
                   for (let i = args.offset; i < limit; ++i) {
-                    existingValue[i] = incomingValue[i - args.offset];
+                    copy[i] = incoming[i - args.offset];
                   }
-                  return existingValue as any;
+                  return copy;
                 }
               },
             },

src/cache/inmemory/policies.ts

@@ -62,7 +62,9 @@ type TypePolicy = {
   subscriptionType?: true,
 
   fields?: {
-    [fieldName: string]: FieldPolicy | FieldReadFunction;
+    [fieldName: string]:
+      | FieldPolicy<StoreValue>
+      | FieldReadFunction<StoreValue>;
   }
 };
 
@@ -75,29 +77,32 @@ type KeyArgsFunction = (
   },
 ) => ReturnType<IdGetter>;
 
-type FieldPolicy<TValue = StoreValue> = {
+export type FieldPolicy<TValue> = {
   keyArgs?: KeySpecifier | KeyArgsFunction;
   read?: FieldReadFunction<TValue>;
   merge?: FieldMergeFunction<TValue>;
 };
 
-interface CommonFieldFunctionOptions {
-  typename: string;
+interface FieldFunctionOptions {
+  args: Record<string, any>;
+  parentObject: Readonly<StoreObject>;
   field: FieldNode;
-  variables: Record<string, any>;
-}
-
-interface FieldReadOptions<TValue> extends CommonFieldFunctionOptions {
-  storeFieldName: string;
-  storeFieldValue: Readonly<TValue>;
+  variables?: Record<string, any>;
 }
 
-interface FieldReadFunction<TValue = StoreValue> {
+interface FieldReadFunction<TExisting, TResult = TExisting> {
   (this: Policies,
-   storeObject: Readonly<StoreObject>,
-   args: Record<string, any>,
-   options: FieldReadOptions<TValue>,
-  ): StoreValue;
+   // When reading a field, one often needs to know about any existing
+   // value stored for that field. If the field is read before any value
+   // has been written to the cache, this existing parameter will be
+   // undefined, which makes it easy to use a default parameter expression
+   // to supply the initial value. This parameter is positional (rather
+   // than one of the named options) because that makes it possible for
+   // the developer to annotate it with a type, without also having to
+   // provide a whole new type for the options object.
+   existing: Readonly<TExisting> | undefined,
+   options: FieldFunctionOptions,
+  ): TResult;
 
   // The TypeScript typings for Function.prototype.call are much too generic
   // to enforce the type safety we need here, for reasons discussed in this
@@ -107,21 +112,26 @@ interface FieldReadFunction<TValue = StoreValue> {
   // https://github.com/microsoft/TypeScript/pull/27028
   call(
     self: Policies,
-    storeObject: Readonly<StoreObject>,
-    args: Record<string, any>,
-    options: FieldReadOptions<TValue>,
-  ): StoreValue;
+    existing: Readonly<TExisting> | undefined,
+    options: FieldFunctionOptions,
+  ): TResult;
 }
 
-interface FieldMergeOptions<TValue> extends CommonFieldFunctionOptions {
-  incomingValue: Readonly<StoreValue>;
-  existingValue?: Readonly<TValue>;
-  args: Record<string, any>;
-}
+interface FieldMergeFunction<TExisting> {
+  (this: Policies,
+   existing: Readonly<TExisting> | undefined,
+   // The incoming parameter needs to be positional as well, for the same
+   // reasons discussed in FieldReadFunction above.
+   incoming: Readonly<StoreValue>,
+   options: FieldFunctionOptions,
+  ): TExisting;
 
-interface FieldMergeFunction<TValue = StoreValue> {
-  (this: Policies, options: FieldMergeOptions<TValue>): TValue;
-  call(self: Policies, options: FieldMergeOptions<TValue>): TValue;
+  call(
+    self: Policies,
+    existing: Readonly<TExisting> | undefined,
+    incoming: Readonly<StoreValue>,
+    options: FieldFunctionOptions,
+  ): TExisting;
 }
 
 export function defaultDataIdFromObject(object: StoreObject) {
@@ -358,46 +368,43 @@ export class Policies {
   }
 
   public readFieldFromStoreObject(
-    typename: string,
-    storeObject: Readonly<StoreObject>,
+    parentObject: Readonly<StoreObject>,
     field: FieldNode,
-    variables: Record<string, any>,
+    typename = parentObject.__typename,
+    variables?: Record<string, any>,
   ): StoreValue {
     const storeFieldName = this.getStoreFieldName(typename, field, variables);
-    const storeFieldValue = storeObject[storeFieldName];
+    const existing = parentObject[storeFieldName];
     const policy = this.getFieldPolicy(typename, field.name.value, false);
     if (policy && policy.read) {
-      // TODO Avoid recomputing this.
-      const args = argumentsObjectFromField(field, variables);
-      return policy.read.call(this, storeObject, args, {
-        typename,
+      return policy.read.call(this, existing, {
+        // TODO Avoid recomputing this.
+        args: argumentsObjectFromField(field, variables),
+        parentObject,
         field,
         variables,
-        storeFieldName,
-        storeFieldValue,
       });
     }
-    return storeFieldValue;
+    return existing;
   }
 
   public getFieldMergeFunction(
     typename: string,
     field: FieldNode,
-    variables: Record<string, any>,
+    variables?: Record<string, any>,
   ): StoreValueMergeFunction {
     const policy = this.getFieldPolicy(typename, field.name.value, false);
     if (policy && policy.merge) {
       return (
-        existingValue: StoreValue,
-        incomingValue: StoreValue,
-      ) => policy.merge.call(this, {
-        typename,
-        field,
-        variables,
-        incomingValue,
-        existingValue,
+        existing: StoreValue,
+        incoming: StoreValue,
+        parentObject: Readonly<StoreObject>,
+      ) => policy.merge.call(this, existing, incoming, {
         // TODO Avoid recomputing this.
         args: argumentsObjectFromField(field, variables),
+        parentObject,
+        field,
+        variables,
       });
     }
   }
@@ -406,6 +413,7 @@ export class Policies {
 export type StoreValueMergeFunction = (
   existing: StoreValue,
   incoming: StoreValue,
+  parentObject: Readonly<StoreObject>,
 ) => StoreValue;
 
 function keyArgsFnFromSpecifier(

src/cache/inmemory/readFromStore.ts

@@ -12,7 +12,6 @@ import {
   isField,
   isInlineFragment,
   resultKeyNameFromField,
-  StoreValue,
   Reference,
   isReference,
   makeReference,
@@ -336,16 +335,8 @@ export class StoreReader {
       policies,
     } = context;
 
-    let fieldValue: StoreValue | undefined;
-
-    if (object) {
-      fieldValue = policies.readFieldFromStoreObject(
-        typename,
-        object,
-        field,
-        variables,
-      );
-    }
+    const fieldValue = object &&
+      policies.readFieldFromStoreObject(object, field, typename, variables);
 
     const readStoreResult = typeof fieldValue === "undefined" ? {
       result: fieldValue,

src/cache/inmemory/writeToStore.ts

@@ -368,10 +368,11 @@ function walkWithMergeOverrides(
     const existingValue: any = existingObject && existingObject[name];
     const incomingValue: any = incomingObject && incomingObject[name];
     if (typeof override === "function") {
-      return incomingObject[name] = override(existingValue, incomingValue);
-    }
-    if (override && typeof override === "object") {
-      return walkWithMergeOverrides(existingValue, incomingValue, override);
+      incomingObject[name] = override(existingValue, incomingValue, existingObject);
+    } else if (override && typeof override === "object") {
+      // StoreObjects can have multiple layers of nested objects/arrays,
+      // each layer with its own nested fields and override functions.
+      walkWithMergeOverrides(existingValue, incomingValue, override);
     }
   });
 }

src/core/__tests__/QueryManager/links.ts

@@ -330,7 +330,7 @@ describe('Link interactions', () => {
         typePolicies: {
           Query: {
             fields: {
-              book(rootQuery, args) {
+              book(_, { parentObject: rootQuery, args }) {
                 const id = this.identify({ __typename: "Book", id: args.id });
                 expect(id).toEqual(`Book:${args.id}`);
                 const found = (rootQuery.books as Reference[]).find(