Merge pull request #5791 from apollographql/reinstate-getCurrentResul…

…t-cache-reads Read from cache again in ObservableQuery#getCurrentResult.
apollographql · Jan 14, 2020 · 6bc9fdc · 6bc9fdc
2 parents 617b1db + b4d8b03
commit 6bc9fdc
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -54,9 +54,6 @@
 - `ApolloClient` is now only available as a named export. The default `ApolloClient` export has been removed. <br/>
   [@hwillson](https://github.com/hwillson) in [#5425](https://github.com/apollographql/apollo-client/pull/5425)
 
-- The `ObservableQuery#getCurrentResult` method no longer falls back to reading from the cache, so calling it immediately after `client.watchQuery` will consistently return a `loading: true` result. When the `fetchPolicy` permits cached results, those results will be delivered via the `next` method of the `ObservableQuery`, and can be obtained by `getCurrentResult` after they have been delivered. This change prevents race conditions where the initial behavior of one query could depend on the timing of cache writes associated with other queries. </br>
-  [@benjamn](https://github.com/benjamn) in [#5565](https://github.com/apollographql/apollo-client/pull/5565)
-
 - The `QueryOptions`, `MutationOptions`, and `SubscriptionOptions` React Apollo interfaces have been renamed to `QueryDataOptions`, `MutationDataOptions`, and `SubscriptionDataOptions` (to avoid conflicting with similarly named and exported Apollo Client interfaces).
 
 - `InMemoryCache` will no longer merge the fields of written objects unless the objects are known to have the same identity, and the values of fields with the same name will not be recursively merged unless a custom `merge` function is defined by a field policy for that field, within a type policy associated with the `__typename` of the parent object. <br/>

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

diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
@@ -20,6 +20,7 @@ import { isNonEmptyArray } from '../utilities/common/arrays';
 
 export type ApolloCurrentQueryResult<T> = ApolloQueryResult<T> & {
   error?: ApolloError;
+  partial?: boolean;
 };
 
 export interface FetchMoreOptions<
@@ -131,8 +132,12 @@ export class ObservableQuery<
   }
 
   public getCurrentResult(): ApolloCurrentQueryResult<TData> {
-    const { lastResult, lastError } = this;
-    const { fetchPolicy } = this.options;
+    const {
+      lastResult,
+      lastError,
+      options: { fetchPolicy },
+    } = this;
+
     const isNetworkFetchPolicy =
       fetchPolicy === 'network-only' ||
       fetchPolicy === 'no-cache';
@@ -145,7 +150,7 @@ export class ObservableQuery<
 
     const result: ApolloCurrentQueryResult<TData> = {
       data: !lastError && lastResult && lastResult.data || void 0,
-      error: this.lastError,
+      error: lastError,
       loading: isNetworkRequestInFlight(networkStatus),
       networkStatus,
       stale: lastResult ? lastResult.stale : false,
@@ -155,6 +160,9 @@ export class ObservableQuery<
       return result;
     }
 
+    const { data, partial } = this.queryManager.getCurrentQueryResult(this);
+    Object.assign(result, { data, partial });
+
     const queryStoreValue = this.queryManager.queryStore.get(this.queryId);
     if (queryStoreValue) {
       const { networkStatus } = queryStoreValue;
@@ -176,11 +184,10 @@ export class ObservableQuery<
       // the original `ObservableQuery`. We'll update the observable query
       // variables here to match, so retrieving from the cache doesn't fail.
       if (queryStoreValue.variables) {
-        this.options.variables = {
+        this.variables = this.options.variables = {
           ...this.options.variables,
           ...(queryStoreValue.variables as TVariables),
         };
-        this.variables = this.options.variables;
       }
 
       Object.assign(result, {
@@ -193,7 +200,10 @@ export class ObservableQuery<
       }
     }
 
-    this.updateLastResult(result);
+    if (!partial) {
+      result.stale = false;
+      this.updateLastResult(result);
+    }
 
     return result;
   }

diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
@@ -1052,7 +1052,7 @@ export class QueryManager<TStore> {
     this.queries.delete(queryId);
   }
 
-  private getCurrentQueryResult<T>(
+  public getCurrentQueryResult<T>(
     observableQuery: ObservableQuery<T>,
     optimistic: boolean = true,
   ): {

diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts
@@ -1450,6 +1450,7 @@ describe('ObservableQuery', () => {
           loading: false,
           networkStatus: 7,
           stale: false,
+          partial: false,
         });
         resolve();
       });
@@ -1459,6 +1460,7 @@ describe('ObservableQuery', () => {
         data: undefined,
         networkStatus: 1,
         stale: false,
+        partial: true,
       });
 
       setTimeout(
@@ -1468,12 +1470,40 @@ describe('ObservableQuery', () => {
             data: undefined,
             networkStatus: 1,
             stale: false,
+            partial: true,
           });
         }),
         0,
       );
     });
 
+    itAsync('returns results from the store immediately', (resolve, reject) => {
+      const queryManager = mockQueryManager(reject, {
+        request: { query, variables },
+        result: { data: dataOne },
+      });
+
+      return queryManager.query({ query, variables }).then((result: any) => {
+        expect(stripSymbols(result)).toEqual({
+          data: dataOne,
+          loading: false,
+          networkStatus: 7,
+          stale: false,
+        });
+        const observable = queryManager.watchQuery({
+          query,
+          variables,
+        });
+        expect(stripSymbols(observable.getCurrentResult())).toEqual({
+          data: dataOne,
+          loading: true,
+          networkStatus: NetworkStatus.loading,
+          stale: false,
+          partial: false,
+        });
+      }).then(resolve, reject);
+    });
+
     itAsync('returns errors from the store immediately', (resolve, reject) => {
       const queryManager = mockQueryManager(reject, {
         request: { query, variables },
@@ -1563,7 +1593,7 @@ describe('ObservableQuery', () => {
       }).then(resolve, reject);
     });
 
-    itAsync('returns partial data from the store', (resolve, reject) => {
+    itAsync('returns partial data from the store immediately', (resolve, reject) => {
       const superQuery = gql`
         query superQuery($id: ID!) {
           people_one(id: $id) {
@@ -1600,10 +1630,11 @@ describe('ObservableQuery', () => {
         });
 
         expect(observable.getCurrentResult()).toEqual({
-          data: void 0,
+          data: dataOne,
           loading: true,
           networkStatus: 1,
           stale: false,
+          partial: true,
         });
 
         // we can use this to trigger the query
@@ -1618,28 +1649,19 @@ describe('ObservableQuery', () => {
 
           if (handleCount === 1) {
             expect(subResult).toEqual({
-              data: void 0,
+              data: dataOne,
               loading: true,
               networkStatus: 1,
               stale: false,
             });
 
           } else if (handleCount === 2) {
-            expect(subResult).toEqual({
-              data: dataOne,
-              loading: false,
-              networkStatus: 7,
-              stale: false,
-            });
-
-          } else if (handleCount === 3) {
             expect(subResult).toEqual({
               data: superDataOne,
               loading: false,
               networkStatus: 7,
               stale: false,
             });
-
             resolve();
           }
         });
@@ -1670,6 +1692,7 @@ describe('ObservableQuery', () => {
           loading: true,
           networkStatus: 1,
           stale: false,
+          partial: false,
         });
 
         subscribeAndCount(reject, observable, (handleCount, subResult) => {
@@ -1679,29 +1702,21 @@ describe('ObservableQuery', () => {
             networkStatus,
           } = observable.getCurrentResult();
 
-          expect(subResult).toEqual({
-            data,
-            loading,
-            networkStatus,
-            stale: false,
-          });
-
           if (handleCount === 1) {
-            expect(stripSymbols(subResult)).toEqual({
-              data: void 0,
-              loading: true,
-              networkStatus: NetworkStatus.loading,
+            expect(subResult).toEqual({
+              data,
+              loading,
+              networkStatus,
               stale: false,
+              partial: false,
             });
-
           } else if (handleCount === 2) {
             expect(stripSymbols(subResult)).toEqual({
               data: dataTwo,
               loading: false,
-              networkStatus: NetworkStatus.ready,
+              networkStatus: 7,
               stale: false,
             });
-
             resolve();
           }
         });
@@ -1727,12 +1742,12 @@ describe('ObservableQuery', () => {
           variables,
           fetchPolicy: 'no-cache',
         });
-
         expect(stripSymbols(observable.getCurrentResult())).toEqual({
           data: undefined,
           loading: true,
           networkStatus: 1,
           stale: false,
+          partial: false,
         });
 
         subscribeAndCount(reject, observable, (handleCount, subResult) => {
@@ -1748,6 +1763,7 @@ describe('ObservableQuery', () => {
               loading,
               networkStatus,
               stale: false,
+              partial: false,
             });
           } else if (handleCount === 2) {
             expect(stripSymbols(subResult)).toEqual({

diff --git a/src/react/data/QueryData.ts b/src/react/data/QueryData.ts
@@ -353,7 +353,7 @@ export class QueryData<TData, TVariables> extends OperationData {
     } else {
       // Fetch the current result (if any) from the store.
       const currentResult = this.currentObservable.query!.getCurrentResult();
-      const { loading, networkStatus, errors } = currentResult;
+      const { loading, partial, networkStatus, errors } = currentResult;
       let { error, data } = currentResult;
 
       // Until a set naming convention for networkError and graphQLErrors is
@@ -390,6 +390,7 @@ export class QueryData<TData, TVariables> extends OperationData {
         const { partialRefetch } = options;
         if (
           partialRefetch &&
+          partial &&
           (!data || Object.keys(data).length === 0) &&
           fetchPolicy !== 'cache-only'
         ) {