Merge pull request #7778 from apollographql/7642-allow-merging-into-e…

…xisting-refs

Allow merge:true to merge references with non-normalized objects, and vice-versa.

CHANGELOG.md

@@ -23,6 +23,9 @@ TBD
 - The `FetchMoreQueryOptions` type now takes two instead of three type parameters (`<TVariables, TData>`), thanks to using `Partial<TVariables>` instead of `K extends typeof TVariables` and `Pick<TVariables, K>`. <br/>
   [@ArnaudBarre](https://github.com/ArnaudBarre) in [#7476](https://github.com/apollographql/apollo-client/pull/7476)
 
+- Allow `merge: true` field policy to merge `Reference` objects with non-normalized objects, and vice-versa. <br/>
+  [@benjamn](https://github.com/benjamn) in [#7778](https://github.com/apollographql/apollo-client/pull/7778)
+
 ### Documentation
 TBD
 

src/cache/inmemory/__tests__/policies.ts

@@ -4294,6 +4294,188 @@ describe("type policies", function () {
       expect(personMergeCount).toBe(3);
     });
 
+    it("can force merging references with non-normalized objects", function () {
+      const nameQuery = gql`
+        query GetName {
+          viewer {
+            name
+          }
+        }
+      `;
+
+      const emailQuery = gql`
+        query GetEmail {
+          viewer {
+            id
+            email
+          }
+        }
+      `;
+
+      check(new InMemoryCache({
+        typePolicies: {
+          Query: {
+            fields: {
+              viewer: {
+                merge: true,
+              },
+            },
+          },
+        },
+      }));
+
+      check(new InMemoryCache({
+        typePolicies: {
+          User: {
+            merge: true,
+          },
+        },
+      }));
+
+      function check(cache: InMemoryCache) {
+        // Write nameQuery first, so the existing data will be a
+        // non-normalized object when we write emailQuery next.
+        cache.writeQuery({
+          query: nameQuery,
+          data: {
+            viewer: {
+              __typename: "User",
+              name: "Alice",
+            },
+          },
+        });
+
+        expect(cache.extract()).toEqual({
+          ROOT_QUERY: {
+            __typename: "Query",
+            viewer: {
+              __typename: "User",
+              name: "Alice",
+            },
+          },
+        });
+
+        cache.writeQuery({
+          query: emailQuery,
+          data: {
+            viewer: {
+              __typename: "User",
+              id: 12345,
+              email: "[email protected]",
+            },
+          },
+        });
+
+        expect(cache.extract()).toEqual({
+          ROOT_QUERY: {
+            __typename: "Query",
+            viewer: {
+              __ref: "User:12345",
+            },
+          },
+          "User:12345": {
+            __typename: "User",
+            name: "Alice",
+            id: 12345,
+            email: "[email protected]",
+          },
+        });
+
+        expect(cache.readQuery({
+          query: nameQuery,
+        })).toEqual({
+          viewer: {
+            __typename: "User",
+            name: "Alice",
+          },
+        });
+
+        expect(cache.readQuery({
+          query: emailQuery,
+        })).toEqual({
+          viewer: {
+            __typename: "User",
+            id: 12345,
+            email: "[email protected]",
+          },
+        });
+
+        cache.reset();
+        expect(cache.extract()).toEqual({});
+
+        // Write emailQuery first, so the existing data will be a
+        // normalized reference when we write nameQuery next.
+        cache.writeQuery({
+          query: emailQuery,
+          data: {
+            viewer: {
+              __typename: "User",
+              id: 12345,
+              email: "[email protected]",
+            },
+          },
+        });
+
+        expect(cache.extract()).toEqual({
+          "User:12345": {
+            id: 12345,
+            __typename: "User",
+            email: "[email protected]"
+          },
+          ROOT_QUERY: {
+            __typename: "Query",
+            viewer: {
+              __ref: "User:12345",
+            },
+          },
+        });
+
+        cache.writeQuery({
+          query: nameQuery,
+          data: {
+            viewer: {
+              __typename: "User",
+              name: "Alice",
+            },
+          },
+        });
+
+        expect(cache.extract()).toEqual({
+          "User:12345": {
+            id: 12345,
+            __typename: "User",
+            email: "[email protected]",
+            name: "Alice",
+          },
+          ROOT_QUERY: {
+            __typename: "Query",
+            viewer: {
+              __ref: "User:12345",
+            },
+          },
+        });
+
+        expect(cache.readQuery({
+          query: nameQuery,
+        })).toEqual({
+          viewer: {
+            __typename: "User",
+            name: "Alice",
+          },
+        });
+
+        expect(cache.readQuery({
+          query: emailQuery,
+        })).toEqual({
+          viewer: {
+            __typename: "User",
+            id: 12345,
+            email: "[email protected]",
+          },
+        });
+      }
+    });
+
     it("can force merging with inherited field merge function", function () {
       let authorMergeCount = 0;
 

src/cache/inmemory/entityStore.ts

@@ -1,4 +1,5 @@
 import { dep, OptimisticDependencyFunction } from 'optimism';
+import invariant from 'ts-invariant';
 import { equal } from '@wry/equality';
 import { Trie } from '@wry/trie';
 
@@ -94,13 +95,38 @@ export abstract class EntityStore implements NormalizedCache {
     }
   }
 
-  public merge(dataId: string, incoming: StoreObject): void {
-    const existing = this.lookup(dataId);
+  public merge(
+    older: string | StoreObject,
+    newer: StoreObject | string,
+  ): void {
+    let dataId: string | undefined;
+
+    const existing: StoreObject | undefined =
+      typeof older === "string"
+        ? this.lookup(dataId = older)
+        : older;
+
+    const incoming: StoreObject | undefined =
+      typeof newer === "string"
+        ? this.lookup(dataId = newer)
+        : newer;
+
+    // If newer was a string ID, but that ID was not defined in this store,
+    // then there are no fields to be merged, so we're done.
+    if (!incoming) return;
+
+    invariant(
+      typeof dataId === "string",
+      "store.merge expects a string ID",
+    );
+
     const merged: StoreObject =
       new DeepMerger(storeObjectReconciler).merge(existing, incoming);
+
     // Even if merged === existing, existing may have come from a lower
     // layer, so we always need to set this.data[dataId] on this level.
     this.data[dataId] = merged;
+
     if (merged !== existing) {
       delete this.refs[dataId];
       if (this.group.caching) {
@@ -142,7 +168,7 @@ export abstract class EntityStore implements NormalizedCache {
         });
 
         Object.keys(fieldsToDirty).forEach(
-          fieldName => this.group.dirty(dataId, fieldName));
+          fieldName => this.group.dirty(dataId as string, fieldName));
       }
     }
   }

src/cache/inmemory/policies.ts

@@ -23,7 +23,12 @@ import {
   canUseWeakMap,
   compact,
 } from '../../utilities';
-import { IdGetter, ReadMergeModifyContext, MergeInfo } from "./types";
+import {
+  IdGetter,
+  MergeInfo,
+  NormalizedCache,
+  ReadMergeModifyContext,
+} from "./types";
 import {
   hasOwn,
   fieldNameFromStoreName,
@@ -41,7 +46,6 @@ import {
   ReadFieldOptions,
   CanReadFunction,
 } from '../core/types/common';
-import { FieldValueGetter } from './entityStore';
 
 export type TypePolicies = {
   [__typename: string]: TypePolicy;
@@ -788,7 +792,7 @@ export class Policies {
       // FieldFunctionOptions object and calling mergeTrueFn, we can
       // simply call mergeObjects, as mergeTrueFn would.
       return makeMergeObjectsFunction(
-        context.store.getFieldValue
+        context.store,
       )(existing as StoreObject,
         incoming as StoreObject);
     }
@@ -832,7 +836,7 @@ function makeFieldFunctionOptions(
   const storeFieldName = policies.getStoreFieldName(fieldSpec);
   const fieldName = fieldNameFromStoreName(storeFieldName);
   const variables = fieldSpec.variables || context.variables;
-  const { getFieldValue, toReference, canRead } = context.store;
+  const { toReference, canRead } = context.store;
 
   return {
     args: argsFromFieldSpecifier(fieldSpec),
@@ -867,12 +871,12 @@ function makeFieldFunctionOptions(
       return policies.readField<T>(options, context);
     },
 
-    mergeObjects: makeMergeObjectsFunction(getFieldValue),
+    mergeObjects: makeMergeObjectsFunction(context.store),
   };
 }
 
 function makeMergeObjectsFunction(
-  getFieldValue: FieldValueGetter,
+  store: NormalizedCache,
 ): MergeObjectsFunction {
   return function mergeObjects(existing, incoming) {
     if (Array.isArray(existing) || Array.isArray(incoming)) {
@@ -885,17 +889,30 @@ function makeMergeObjectsFunction(
     // 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 = store.getFieldValue(existing, "__typename");
+      const iType = store.getFieldValue(incoming, "__typename");
       const typesDiffer = eType && iType && eType !== iType;
 
-      if (typesDiffer ||
-          !storeValueIsStoreObject(existing) ||
-          !storeValueIsStoreObject(incoming)) {
+      if (typesDiffer) {
         return incoming;
       }
 
-      return { ...existing, ...incoming };
+      if (isReference(existing) &&
+          storeValueIsStoreObject(incoming)) {
+        store.merge(existing.__ref, incoming);
+        return existing;
+      }
+
+      if (storeValueIsStoreObject(existing) &&
+          isReference(incoming)) {
+        store.merge(existing, incoming.__ref);
+        return incoming;
+      }
+
+      if (storeValueIsStoreObject(existing) &&
+          storeValueIsStoreObject(incoming)) {
+        return { ...existing, ...incoming };
+      }
     }
 
     return incoming;

src/cache/inmemory/types.ts

@@ -33,7 +33,13 @@ export declare type IdGetter = (
 export interface NormalizedCache {
   has(dataId: string): boolean;
   get(dataId: string, fieldName: string): StoreValue;
-  merge(dataId: string, incoming: StoreObject): void;
+
+  // The store.merge method allows either argument to be a string ID, but
+  // the other argument has to be a StoreObject. Either way, newer fields
+  // always take precedence over older fields.
+  merge(olderId: string, newerObject: StoreObject): void;
+  merge(olderObject: StoreObject, newerId: string): void;
+
   modify(dataId: string, fields: Modifiers | Modifier<any>): boolean;
   delete(dataId: string, fieldName?: string): boolean;
   clear(): void;