Merge pull request #6058 from apollographql/stop-delivering-stale-res…

…ults Stop delivering "stale" results to ObservableQuery.
apollographql · Apr 6, 2020 · 963b7c2 · 963b7c2
2 parents 0aacba4 + 70ae548
commit 963b7c2
Show file tree

Hide file tree

Showing 22 changed files with 280 additions and 269 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -44,6 +44,9 @@
 - **[BREAKING]** `client|cache.writeData` have been fully removed. `writeData` usage is one of the easiest ways to turn faulty assumptions about how the cache represents data internally, into cache inconsistency and corruption. `client|cache.writeQuery`, `client|cache.writeFragment`, and/or `cache.modify` can be used to update the cache.  <br/>
   [@benjamn](https://github.com/benjamn) in [#5923](https://github.com/apollographql/apollo-client/pull/5923)
 
+- **[BREAKING]** Apollo Client will no longer deliver "stale" results to `ObservableQuery` consumers, but will instead log more helpful errors about which cache fields were missing. <br/>
+  [@benjamn](https://github.com/benjamn) in [#6058](https://github.com/apollographql/apollo-client/pull/6058)
+
 - `InMemoryCache` now supports tracing garbage collection and eviction. Note that the signature of the `evict` method has been simplified in a potentially backwards-incompatible way. <br/>
   [@benjamn](https://github.com/benjamn) in [#5310](https://github.com/apollographql/apollo-client/pull/5310)
 

diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts
@@ -1654,14 +1654,10 @@ describe('client', () => {
       });
 
       subscribeAndCount(reject, obs, (handleCount, result) => {
-        if (handleCount === 1) {
-          expect(result.data).toBe(undefined);
-          expect(result.loading).toBe(true);
-        } else if (handleCount === 2) {
-          expect(stripSymbols(result.data)).toEqual(networkFetch);
-          expect(result.loading).toBe(false);
-          resolve();
-        }
+        expect(handleCount).toBe(1);
+        expect(stripSymbols(result.data)).toEqual(networkFetch);
+        expect(result.loading).toBe(false);
+        resolve();
       });
     });
 
@@ -1677,17 +1673,13 @@ describe('client', () => {
         fetchPolicy: 'cache-and-network',
       });
 
-      let count = 0;
       obs.subscribe({
-        next: result => {
-          expect(result.data).toBe(undefined);
-          expect(result.loading).toBe(true);
-          count++;
-        },
         error: e => {
-          expect(e.message).toMatch(/No more mocked responses/);
-          expect(count).toBe(1); // make sure next was called.
-          setTimeout(resolve, 100);
+          if (!/No more mocked responses/.test(e.message)) {
+            reject(e);
+          } else {
+            resolve();
+          }
         },
       });
     });

diff --git a/src/__tests__/local-state/general.ts b/src/__tests__/local-state/general.ts
@@ -422,7 +422,6 @@ describe('Cache manipulation', () => {
           },
           loading: false,
           networkStatus: 7,
-          stale: false,
         });
 
         if (selectedItemId !== 123) {

diff --git a/src/__tests__/subscribeToMore.ts b/src/__tests__/subscribeToMore.ts
@@ -101,7 +101,6 @@ describe('subscribeToMore', () => {
         data: { entry: { value: 'Amanda Liu' } },
         loading: false,
         networkStatus: 7,
-        stale: false,
       });
       resolve();
     }, 15);
@@ -157,7 +156,6 @@ describe('subscribeToMore', () => {
         data: { entry: { value: 'Amanda Liu' } },
         loading: false,
         networkStatus: 7,
-        stale: false,
       });
       expect(counter).toBe(2);
       expect(errorCount).toBe(1);
@@ -217,7 +215,6 @@ describe('subscribeToMore', () => {
         data: { entry: { value: '1' } },
         loading: false,
         networkStatus: 7,
-        stale: false,
       });
       expect(counter).toBe(1);
       expect(errorCount).toBe(1);
@@ -323,7 +320,6 @@ describe('subscribeToMore', () => {
       },
       loading: false,
       networkStatus: 7,
-      stale: false,
     });
     resolve();
   });
@@ -391,7 +387,6 @@ describe('subscribeToMore', () => {
         data: { entry: { value: 'Amanda Liu' } },
         loading: false,
         networkStatus: 7,
-        stale: false,
       });
       resolve();
     }, 15);

diff --git a/src/cache/core/types/DataProxy.ts b/src/cache/core/types/DataProxy.ts
@@ -1,5 +1,7 @@
 import { DocumentNode } from 'graphql'; // eslint-disable-line import/no-extraneous-dependencies, import/no-unresolved
 
+import { MissingFieldError } from './common';
+
 export namespace DataProxy {
   export interface Query<TVariables> {
     /**
@@ -70,7 +72,8 @@ export namespace DataProxy {
   export type DiffResult<T> = {
     result?: T;
     complete?: boolean;
-  };
+    missing?: MissingFieldError[];
+  }
 }
 
 /**

diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts
@@ -1,3 +1,5 @@
+import { DocumentNode } from 'graphql';
+
 import {
   isReference,
   StoreValue,
@@ -31,3 +33,12 @@ export type Modifier<T> = (value: T, details: {
 export type Modifiers = {
   [fieldName: string]: Modifier<any>;
 }
+
+export class MissingFieldError {
+  constructor(
+    public readonly message: string,
+    public readonly path: (string | number)[],
+    public readonly query: DocumentNode,
+    public readonly variables: Record<string, any>,
+  ) {}
+}
diff --git a/src/cache/index.ts b/src/cache/index.ts
@@ -1,6 +1,7 @@
 export { Transaction, ApolloCache } from './core/cache';
 export { Cache } from './core/types/Cache';
 export { DataProxy } from './core/types/DataProxy';
+export { MissingFieldError } from './core/types/common';
 
 export {
   Reference,

diff --git a/src/cache/inmemory/__tests__/entityStore.ts b/src/cache/inmemory/__tests__/entityStore.ts
@@ -6,6 +6,7 @@ import { Policies } from '../policies';
 import { StoreObject } from '../types';
 import { ApolloCache } from '../../core/cache';
 import { Reference } from '../../../utilities/graphql/storeUtils';
+import { MissingFieldError } from '../..';
 
 describe('EntityStore', () => {
   it('should support result caching if so configured', () => {
@@ -1065,6 +1066,20 @@ describe('EntityStore', () => {
       result: {
         authorOfBook: tedWithoutHobby,
       },
+      missing: [
+        new MissingFieldError(
+          'Can\'t find field \'hobby\' on Author:{"name":"Ted Chiang"} object',
+          ["authorOfBook", "hobby"],
+          expect.anything(),
+          expect.anything(),
+        ),
+        new MissingFieldError(
+          'Can\'t find field \'publisherOfBook\' on ROOT_QUERY object',
+          ["publisherOfBook"],
+          expect.anything(),
+          expect.anything(),
+        ),
+      ],
     });
 
     cache.evict("ROOT_QUERY", "authorOfBook");
@@ -1296,12 +1311,22 @@ describe('EntityStore', () => {
 
     expect(cache.evict(authorId)).toBe(false);
 
+    const missing = [
+      new MissingFieldError(
+        "Dangling reference to missing Author:2 object",
+        ["book", "author"],
+        expect.anything(),
+        expect.anything(),
+      ),
+    ];
+
     expect(cache.diff({
       query,
       optimistic: true,
       returnPartialData: true,
     })).toEqual({
       complete: false,
+      missing,
       result: {
         book: {
           __typename: "Book",
@@ -1331,6 +1356,7 @@ describe('EntityStore', () => {
       returnPartialData: true,
     })).toEqual({
       complete: false,
+      missing,
       result: {
         book: {
           __typename: "Book",
@@ -1553,6 +1579,14 @@ describe('EntityStore', () => {
           isbn: "031648637X",
         },
       },
+      missing: [
+        new MissingFieldError(
+          'Can\'t find field \'title\' on Book:{"isbn":"031648637X"} object',
+          ["book", "title"],
+          expect.anything(),
+          expect.anything(),
+        ),
+      ],
     });
 
     expect(cache.extract()).toEqual({

diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts
@@ -3,6 +3,7 @@ import gql from "graphql-tag";
 import { InMemoryCache } from "../inMemoryCache";
 import { Policies } from "../policies";
 import { Reference, StoreObject } from "../../../core";
+import { MissingFieldError } from "../..";
 
 function reverse(s: string) {
   return s.split("").reverse().join("");
@@ -997,6 +998,15 @@ describe("type policies", function () {
 
       expect(cache.extract()).toEqual(snapshot1);
 
+      function makeMissingError(jobNumber: number) {
+        return new MissingFieldError(
+          `Can't find field 'result' on Job:{"name":"Job #${jobNumber}"} object`,
+          ["jobs", jobNumber - 1, "result"],
+          expect.anything(),
+          expect.anything(),
+        );
+      }
+
       expect(cache.diff({
         query,
         optimistic: false,
@@ -1015,6 +1025,11 @@ describe("type policies", function () {
           }],
         },
         complete: false,
+        missing: [
+          makeMissingError(1),
+          makeMissingError(2),
+          makeMissingError(3),
+        ],
       });
 
       function setResult(jobNum: number) {
@@ -1061,6 +1076,10 @@ describe("type policies", function () {
           }],
         },
         complete: false,
+        missing: [
+          makeMissingError(1),
+          makeMissingError(3),
+        ],
       });
 
       cache.writeQuery({
@@ -1114,6 +1133,10 @@ describe("type policies", function () {
           }],
         },
         complete: false,
+        missing: [
+          makeMissingError(1),
+          makeMissingError(3),
+        ],
       });
 
       setResult(1);