Merge pull request #6714 from apollographql/ergonomic-improvements-fo…

…r-field-policies

Ergonomic improvements for field policy merge and keyArgs functions.

CHANGELOG.md

@@ -27,6 +27,9 @@
 - Allow passing an asynchronous `options.renderFunction` to `getMarkupFromTree`. <br/>
   [@richardscarrott](https://github.com/richardscarrott) in [#6576](https://github.com/apollographql/apollo-client/pull/6576)
 
+- Ergonomic improvements for `merge` and `keyArgs` functions in cache field policies. <br/>
+  [@benjamn](https://github.com/benjamn) in [#6714](https://github.com/apollographql/apollo-client/pull/6714)
+
 ## Apollo Client 3.0.2
 
 ## Bug Fixes

docs/source/caching/cache-field-behavior.md

@@ -208,6 +208,23 @@ const cache = new InMemoryCache({
 });
 ```
 
+Since writing this kind of `merge` function can become repetitive, the following shorthand will provide the same behavior:
+
+```ts
+const cache = new InMemoryCache({
+  typePolicies: {
+    Book: {
+      fields: {
+        author: {
+          // Short for always preferring incoming over existing data.
+          merge: false,
+        },
+      },
+    },
+  },
+});
+```
+
 When you use `{ ...existing, ...incoming }`, `Author` objects with differing fields (`name`, `dateOfBirth`) can be combined without losing fields, which is definitely an improvement over blind replacement.
 
 But what if the `Author` type defines its own custom `merge` functions for fields of the `incoming` object? Since we're using [object spread syntax](https://2ality.com/2016/10/rest-spread-properties.html), such fields will immediately overwrite fields in `existing`, without triggering any nested `merge` functions. The `{ ...existing, ...incoming }` syntax may be an improvement, but it is not fully correct.
@@ -233,6 +250,23 @@ const cache = new InMemoryCache({
 
 Because this `Book.author` field policy has no `Book`- or `Author`-specific logic in it, you can reuse this `merge` function for any field that needs this kind of handling.
 
+Since writing this kind of `merge` function can become repetitive, the following shorthand will provide the same behavior:
+
+```ts
+const cache = new InMemoryCache({
+  typePolicies: {
+    Book: {
+      fields: {
+        author: {
+          // Short for options.mergeObjects(existing, incoming).
+          merge: true,
+        },
+      },
+    },
+  },
+});
+```
+
 In summary, the `Book.author` policy above allows the cache to safely merge the `author` objects of any two `Book` objects that have the same identity.
 
 ### Merging arrays of non-normalized objects
@@ -475,6 +509,7 @@ An example of a _non-key_ argument is an access token, which is used to authoriz
 
 By default, the cache stores a separate value for _every unique combination of argument values you provide when querying a particular field_. When you specify a field's key arguments, the cache understands that any _non_-key arguments don't affect that field's value. Consequently, if you execute two different queries with the `monthForNumber` field, passing the _same_ `number` argument but _different_ `accessToken` arguments, the second query response will overwrite the first, because both invocations use the exact same value for all key arguments.
 
+If you need more control over the behavior of `keyArgs`, you can pass a function instead of an array. This `keyArgs` function will receive the arguments object as its first parameter, and a `context` object providing other relevant details as its second parameter. See `KeyArgsFunction` in the types below for further information.
 
 ## `FieldPolicy` API reference
 
@@ -490,7 +525,7 @@ type FieldPolicy<
 > = {
   keyArgs?: KeySpecifier | KeyArgsFunction | false;
   read?: FieldReadFunction<TExisting, TReadResult>;
-  merge?: FieldMergeFunction<TExisting, TIncoming>;
+  merge?: FieldMergeFunction<TExisting, TIncoming> | boolean;
 };
 
 type KeySpecifier = (string | KeySpecifier)[];
@@ -501,6 +536,7 @@ type KeyArgsFunction = (
     typename: string;
     fieldName: string;
     field: FieldNode | null;
+    variables?: Record<string, any>;
   },
 ) => string | KeySpecifier | null | void;
 

src/__tests__/ApolloClient.ts

@@ -679,9 +679,7 @@ describe('ApolloClient', () => {
                 d: {
                   // Silence "Cache data may be lost..."  warnings by
                   // unconditionally favoring the incoming data.
-                  merge(_, incoming) {
-                    return incoming;
-                  },
+                  merge: false,
                 },
               },
             },
@@ -1196,9 +1194,7 @@ describe('ApolloClient', () => {
                     // Deliberately silence "Cache data may be lost..."
                     // warnings by preferring the incoming data, rather
                     // than (say) concatenating the arrays together.
-                    merge(_, incoming) {
-                      return incoming;
-                    },
+                    merge: false,
                   },
                 },
               },

src/__tests__/fetchMore.ts

@@ -2,7 +2,7 @@ import { assign, cloneDeep } from 'lodash';
 import gql from 'graphql-tag';
 
 import { itAsync, mockSingleLink, subscribeAndCount } from '../testing';
-import { InMemoryCache, InMemoryCacheConfig } from '../cache';
+import { InMemoryCache, InMemoryCacheConfig, FieldMergeFunction } from '../cache';
 import { ApolloClient, NetworkStatus, ObservableQuery } from '../core';
 import { offsetLimitPagination, concatPagination } from '../utilities';
 
@@ -215,9 +215,7 @@ describe('fetchMore on an observable query', () => {
           Query: {
             fields: {
               entry: {
-                merge(_, incoming) {
-                  return incoming;
-                },
+                merge: false,
               },
             },
           },
@@ -414,7 +412,8 @@ describe('fetchMore on an observable query', () => {
     const { merge } = groceriesFieldPolicy;
     groceriesFieldPolicy.merge = function (existing, incoming, options) {
       mergeArgsHistory.push(options.args);
-      return merge!.call(this, existing, incoming, options);
+      return (merge as FieldMergeFunction<any>).call(
+        this, existing, incoming, options);
     };
 
     const cache = new InMemoryCache({
@@ -831,9 +830,7 @@ describe('fetchMore on an observable query with connection', () => {
           Query: {
             fields: {
               entry: {
-                merge(_, incoming) {
-                  return incoming;
-                },
+                merge: false,
               },
             },
           },

src/__tests__/optimistic.ts

@@ -135,9 +135,7 @@ describe('optimistic mutation results', () => {
                 // Deliberately silence "Cache data may be lost..."
                 // warnings by favoring the incoming data, rather than
                 // (say) concatenating the arrays together.
-                merge(_, incoming) {
-                  return incoming;
-                },
+                merge: false,
               },
             },
           },

src/cache/inmemory/__tests__/cache.ts

@@ -876,9 +876,7 @@ describe('Cache', () => {
               d: {
                 // Deliberately silence "Cache data may be lost..."
                 // warnings by unconditionally favoring the incoming data.
-                merge(_, incoming) {
-                  return incoming;
-                },
+                merge: false,
               },
             },
           },

src/cache/inmemory/__tests__/policies.ts

@@ -624,6 +624,9 @@ describe("type policies", function () {
                   expect(context.typename).toBe("Thread");
                   expect(context.fieldName).toBe("comments");
                   expect(context.field!.name.value).toBe("comments");
+                  expect(context.variables).toEqual({
+                    unused: "check me",
+                  });
 
                   if (typeof args!.limit === "number") {
                     if (typeof args!.offset === "number") {
@@ -682,6 +685,9 @@ describe("type policies", function () {
             }],
           },
         },
+        variables: {
+          unused: "check me",
+        },
       });
 
       expect(cache.extract()).toEqual({
@@ -3267,6 +3273,10 @@ describe("type policies", function () {
         },
       });
 
+      testForceMerges(cache);
+    });
+
+    function testForceMerges(cache: InMemoryCache) {
       const queryWithAuthorName = gql`
         query {
           currentlyReading {
@@ -3443,6 +3453,56 @@ describe("type policies", function () {
           },
         },
       });
+    }
+
+    // Same as previous test, except with merge:true for Book.author.
+    it("can force merging with merge: true", function () {
+      const cache = new InMemoryCache({
+        typePolicies: {
+          Book: {
+            keyFields: ["isbn"],
+            fields: {
+              author: {
+                merge: true,
+              },
+            },
+          },
+
+          Author: {
+            keyFields: false,
+            fields: {
+              books: {
+                merge(existing: any[], incoming: any[], {
+                  isReference,
+                }) {
+                  const merged = existing ? existing.slice(0) : [];
+                  const seen = new Set<string>();
+                  if (existing) {
+                    existing.forEach(book => {
+                      if (isReference(book)) {
+                        seen.add(book.__ref);
+                      }
+                    });
+                  }
+                  incoming.forEach(book => {
+                    if (isReference(book)) {
+                      if (!seen.has(book.__ref)) {
+                        merged.push(book);
+                        seen.add(book.__ref);
+                      }
+                    } else {
+                      merged.push(book);
+                    }
+                  });
+                  return merged;
+                },
+              },
+            },
+          },
+        },
+      });
+
+      testForceMerges(cache);
     });
   });
 

src/cache/inmemory/policies.ts

@@ -88,6 +88,7 @@ export type KeyArgsFunction = (
     typename: string;
     fieldName: string;
     field: FieldNode | null;
+    variables?: Record<string, any>;
   },
 ) => KeySpecifier | ReturnType<IdGetter>;
 
@@ -98,7 +99,7 @@ export type FieldPolicy<
 > = {
   keyArgs?: KeySpecifier | KeyArgsFunction | false;
   read?: FieldReadFunction<TExisting, TReadResult>;
-  merge?: FieldMergeFunction<TExisting, TIncoming>;
+  merge?: FieldMergeFunction<TExisting, TIncoming> | boolean;
 };
 
 type StorageType = Record<string, any>;
@@ -212,6 +213,12 @@ export const defaultDataIdFromObject = (
 const nullKeyFieldsFn: KeyFieldsFunction = () => void 0;
 const simpleKeyArgsFn: KeyArgsFunction = (_args, context) => context.fieldName;
 
+// These merge functions can be selected by specifying merge:true or
+// merge:false in a field policy.
+const mergeTrueFn: FieldMergeFunction<any> =
+  (existing, incoming, { mergeObjects }) => mergeObjects(existing, incoming);
+const mergeFalseFn: FieldMergeFunction<any> = (_, incoming) => incoming;
+
 export type PossibleTypesMap = {
   [supertype: string]: string[];
 };
@@ -349,7 +356,16 @@ export class Policies {
               existing.keyFn;
 
             if (typeof read === "function") existing.read = read;
-            if (typeof merge === "function") existing.merge = merge;
+
+            existing.merge =
+              typeof merge === "function" ? merge :
+              // Pass merge:true as a shorthand for a merge implementation
+              // that returns options.mergeObjects(existing, incoming).
+              merge === true ? mergeTrueFn :
+              // Pass merge:false to make incoming always replace existing
+              // without any warnings about data clobbering.
+              merge === false ? mergeFalseFn :
+              existing.merge;
           }
 
           if (existing.read && existing.merge) {
@@ -477,6 +493,7 @@ export class Policies {
         typename,
         fieldName,
         field: fieldSpec.field || null,
+        variables: fieldSpec.variables,
       };
       const args = argsFromFieldSpecifier(fieldSpec);
       while (keyFn) {

src/core/__tests__/QueryManager/index.ts

@@ -2167,9 +2167,7 @@ describe('QueryManager', () => {
         Query: {
           fields: {
             info: {
-              merge(_, incoming) {
-                return incoming;
-              },
+              merge: false,
             },
           },
         },