Merge pull request #5684 from apollographql/eliminate-heuristic-fragm…

…ent-matching

Fully eliminate the concept of heuristic fragment matching.

package.json

@@ -48,7 +48,7 @@
     {
       "name": "apollo-client",
       "path": "./dist/apollo-client.cjs.min.js",
-      "maxSize": "24.05 kB"
+      "maxSize": "24 kB"
     }
   ],
   "peerDependencies": {

src/cache/inmemory/__tests__/diffAgainstStore.ts

@@ -258,7 +258,7 @@ describe('diffing queries against the store', () => {
         returnPartialData: false,
       });
 
-      expect(complete).toBe(false);
+      expect(complete).toBe(true);
     });
   });
 
@@ -306,7 +306,7 @@ describe('diffing queries against the store', () => {
       query: unionQuery,
     });
 
-    expect(complete).toBe(false);
+    expect(complete).toBe(true);
   });
 
   it('throws an error on a query with fields missing from matching named fragments', () => {

src/cache/inmemory/__tests__/entityStore.ts

@@ -1199,10 +1199,16 @@ describe('EntityStore', () => {
 
     expect(() => cache.readQuery({
       query: queryWithAliases,
-    })).toThrow(/Can't find field a on object/);
+    })).toThrow(
+      "Attempted to match fragment Rest with type condition ABCs against " +
+        "object with unknown __typename"
+    );
 
     expect(() => cache.readQuery({
       query: queryWithoutAliases,
-    })).toThrow(/Can't find field a on object/);
+    })).toThrow(
+      "Attempted to match fragment Rest with type condition ABCs against " +
+        "object with unknown __typename"
+    );
   });
 });

src/cache/inmemory/__tests__/roundtrip.ts

@@ -21,7 +21,12 @@ function assertDeeplyFrozen(value: any, stack: any[] = []) {
 }
 
 function storeRoundtrip(query: DocumentNode, result: any, variables = {}) {
-  const policies = new Policies();
+  const policies = new Policies({
+    possibleTypes: {
+      Character: ["Jedi", "Droid"],
+    },
+  });
+
   const reader = new StoreReader({ policies });
   const writer = new StoreWriter({ policies });
 
@@ -303,8 +308,8 @@ describe('roundtrip', () => {
       );
     });
 
-    it('should resolve on union types with inline fragments without typenames with warning', () => {
-      return withWarning(() => {
+    it('should error on union types with inline fragments without typenames', () => {
+      return expect(() => {
         storeRoundtrip(
           gql`
             query {
@@ -332,13 +337,16 @@ describe('roundtrip', () => {
             ],
           },
         );
-      });
+      }).toThrowError(
+        'Attempted to match fragment with type condition Jedi against ' +
+          'object with unknown __typename'
+      );
     });
 
     // XXX this test is weird because it assumes the server returned an incorrect result
     // However, the user may have written this result with client.writeQuery.
     it('should throw an error on two of the same inline fragment types', () => {
-      return expect(() => {
+      return withWarning(() => expect(() => {
         storeRoundtrip(
           gql`
             query {
@@ -364,7 +372,7 @@ describe('roundtrip', () => {
             ],
           },
         );
-      }).toThrowError(/Can\'t find field rank on object/);
+      }).toThrowError(/Can\'t find field rank on object/));
     });
 
     it('should resolve fields it can on interface with non matching inline fragments', () => {
@@ -469,6 +477,7 @@ describe('roundtrip', () => {
                 __typename: 'Droid',
                 name: 'R2D2',
                 model: 'astromech',
+                side: 'bright',
               },
             ],
           },
@@ -477,7 +486,7 @@ describe('roundtrip', () => {
     });
 
     it('should throw on error on two of the same spread fragment types', () => {
-      expect(() =>
+      withWarning(() => expect(() => {
         storeRoundtrip(
           gql`
             fragment jediSide on Jedi {
@@ -506,8 +515,8 @@ describe('roundtrip', () => {
               },
             ],
           },
-        ),
-      ).toThrowError(/Can\'t find field rank on object/);
+        );
+      }).toThrowError(/Can\'t find field rank on object/));
     });
 
     it('should resolve on @include and @skip with inline fragments', () => {

src/cache/inmemory/__tests__/writeToStore.ts

@@ -1546,7 +1546,7 @@ describe('writing to the store', () => {
       expect(newStore.get('1')).toEqual(result.todos[0]);
     });
 
-    it('should warn when it receives the wrong data with non-union fragments (using an heuristic matcher)', () => {
+    it('should warn when it receives the wrong data with non-union fragments', () => {
       const result = {
         todos: [
           {
@@ -1573,7 +1573,7 @@ describe('writing to the store', () => {
       }, /Missing field description/);
     });
 
-    it('should warn when it receives the wrong data inside a fragment (using an introspection matcher)', () => {
+    it('should warn when it receives the wrong data inside a fragment', () => {
       const queryWithInterface = gql`
         query {
           todos {
@@ -1627,7 +1627,7 @@ describe('writing to the store', () => {
       }, /Missing field price/);
     });
 
-    it('should warn if a result is missing __typename when required (using an heuristic matcher)', () => {
+    it('should warn if a result is missing __typename when required', () => {
       const result: any = {
         todos: [
           {

src/cache/inmemory/policies.ts

@@ -377,10 +377,18 @@ export class Policies {
   public fragmentMatches(
     fragment: InlineFragmentNode | FragmentDefinitionNode,
     typename: string,
-  ): boolean | "heuristic" {
+  ): boolean {
     if (!fragment.typeCondition) return true;
 
     const supertype = fragment.typeCondition.name.value;
+
+    invariant(
+      typename,
+      `Attempted to match fragment ${
+        fragment.kind === "InlineFragment" ? "" : fragment.name.value + " "
+      }with type condition ${supertype} against object with unknown __typename`,
+    );
+
     if (typename === supertype) return true;
 
     if (this.usingPossibleTypes) {
@@ -399,12 +407,9 @@ export class Policies {
           });
         }
       }
-      // When possibleTypes is defined, we always either return true from the
-      // loop above or return false here (never 'heuristic' below).
-      return false;
     }
 
-    return "heuristic";
+    return false;
   }
 
   public getStoreFieldName(

src/cache/inmemory/readFromStore.ts

@@ -6,7 +6,7 @@ import {
   SelectionSetNode,
 } from 'graphql';
 import { wrap } from 'optimism';
-import { invariant } from 'ts-invariant';
+import { invariant, InvariantError } from 'ts-invariant';
 
 import {
   isField,
@@ -51,7 +51,6 @@ interface ExecContext {
 export type ExecResultMissingField = {
   object: StoreObject;
   fieldName: string;
-  tolerable: boolean;
 };
 
 export type ExecResult<R = any> = {
@@ -178,14 +177,11 @@ export class StoreReader {
 
     if (hasMissingFields && ! returnPartialData) {
       execResult.missing!.forEach(info => {
-        invariant(
-          info.tolerable,
-          `Can't find field ${info.fieldName} on object ${JSON.stringify(
-            info.object,
-            null,
-            2,
-          )}.`,
-        );
+        throw new InvariantError(`Can't find field ${
+          info.fieldName
+        } on object ${
+          JSON.stringify(info.object, null, 2)
+        }.`);
       });
     }
 
@@ -244,7 +240,6 @@ export class StoreReader {
           getMissing().push({
             object: objectOrReference as StoreObject,
             fieldName: selection.name.value,
-            tolerable: false,
           });
 
         } else if (Array.isArray(fieldValue)) {
@@ -298,25 +293,14 @@ export class StoreReader {
           );
         }
 
-        const match = policies.fragmentMatches(fragment, typename);
-
-        if (match) {
-          let fragmentExecResult = this.executeSelectionSet({
-            selectionSet: fragment.selectionSet,
-            objectOrReference,
-            context,
-          });
-
-          if (match === 'heuristic' && fragmentExecResult.missing) {
-            fragmentExecResult = {
-              ...fragmentExecResult,
-              missing: fragmentExecResult.missing.map(info => {
-                return { ...info, tolerable: true };
-              }),
-            };
-          }
-
-          objectsToMerge.push(handleMissing(fragmentExecResult));
+        if (policies.fragmentMatches(fragment, typename)) {
+          objectsToMerge.push(handleMissing(
+            this.executeSelectionSet({
+              selectionSet: fragment.selectionSet,
+              objectOrReference,
+              context,
+            })
+          ));
         }
       }
     });

src/cache/inmemory/writeToStore.ts

@@ -135,11 +135,20 @@ export class StoreWriter {
     if (sets.indexOf(selectionSet) >= 0) return store;
     sets.push(selectionSet);
 
+    const typename =
+      // If the result has a __typename, trust that.
+      getTypenameFromResult(result, selectionSet, context.fragmentMap) ||
+      // If the entity identified by dataId has a __typename in the store,
+      // fall back to that.
+      store.getFieldValue(dataId, "__typename") as string ||
+      // Special dataIds like ROOT_QUERY have a known default __typename.
+      this.policies.rootTypenamesById[dataId];
+
     const processed = this.processSelectionSet({
       result,
       selectionSet,
       context,
-      typename: this.policies.rootTypenamesById[dataId],
+      typename,
     });
 
     if (processed.mergeOverrides) {
@@ -165,14 +174,13 @@ export class StoreWriter {
     selectionSet,
     context,
     mergeOverrides,
-    typename = getTypenameFromResult(
-      result, selectionSet, context.fragmentMap),
+    typename,
   }: {
     result: Record<string, any>;
     selectionSet: SelectionSetNode;
     context: WriteContext;
     mergeOverrides?: MergeOverrides;
-    typename?: string;
+    typename: string;
   }): {
     result: StoreObject;
     mergeOverrides?: MergeOverrides;
@@ -328,6 +336,8 @@ export class StoreWriter {
       result: value,
       selectionSet: field.selectionSet,
       context,
+      typename: getTypenameFromResult(
+        value, field.selectionSet, context.fragmentMap),
     });
   }
 }

src/utilities/graphql/storeUtils.ts

@@ -265,6 +265,10 @@ export function getTypenameFromResult(
   selectionSet: SelectionSetNode,
   fragmentMap: FragmentMap,
 ): string | undefined {
+  if (typeof result.__typename === 'string') {
+    return result.__typename;
+  }
+
   for (const selection of selectionSet.selections) {
     if (isField(selection)) {
       if (selection.name.value === '__typename') {
@@ -281,10 +285,6 @@ export function getTypenameFromResult(
       }
     }
   }
-  // TODO Move this first?
-  if (typeof result.__typename === 'string') {
-    return result.__typename;
-  }
 }
 
 export function isField(selection: SelectionNode): selection is FieldNode {