Merge pull request #5565 from apollographql/avoid-reading-from-cache-…

…in-getCurrentResult Avoid reading from cache in ObservableQuery#getCurrentResult.
apollographql · Nov 12, 2019 · 9892d35 · 9892d35
2 parents c0f82b4 + 2785d75
commit 9892d35
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 135 deletions.
diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
@@ -1,4 +1,3 @@
-import { GraphQLError } from 'graphql';
 import { invariant, InvariantError } from 'ts-invariant';
 
 import { isEqual } from '../utilities/common/isEqual';
@@ -19,14 +18,8 @@ import {
 import { QueryStoreValue } from '../data/queries';
 import { isNonEmptyArray } from '../utilities/common/arrays';
 
-export type ApolloCurrentQueryResult<T> = {
-  data: T | undefined;
-  errors?: ReadonlyArray<GraphQLError>;
-  loading: boolean;
-  networkStatus: NetworkStatus;
+export type ApolloCurrentQueryResult<T> = ApolloQueryResult<T> & {
   error?: ApolloError;
-  partial?: boolean;
-  stale?: boolean;
 };
 
 export interface FetchMoreOptions<
@@ -137,46 +130,44 @@ export class ObservableQuery<
     });
   }
 
-  /**
-   * Return the result of the query from the local cache as well as some fetching status
-   * `loading` and `networkStatus` allow to know if a request is in flight
-   * `partial` lets you know if the result from the local cache is complete or partial
-   * @return {data: Object, error: ApolloError, loading: boolean, networkStatus: number, partial: boolean}
-   */
   public getCurrentResult(): ApolloCurrentQueryResult<TData> {
-    if (this.isTornDown) {
-      const { lastResult } = this;
-      return {
-        data: !this.lastError && lastResult && lastResult.data || void 0,
-        error: this.lastError,
-        loading: false,
-        networkStatus: NetworkStatus.error,
-      };
-    }
-
-    const { data, partial } = this.queryManager.getCurrentQueryResult(this);
-    const queryStoreValue = this.queryManager.queryStore.get(this.queryId);
-    let result: ApolloQueryResult<TData>;
-
+    const { lastResult, lastError } = this;
     const { fetchPolicy } = this.options;
-
     const isNetworkFetchPolicy =
       fetchPolicy === 'network-only' ||
       fetchPolicy === 'no-cache';
 
+    const networkStatus =
+      lastError ? NetworkStatus.error :
+      lastResult ? lastResult.networkStatus :
+      isNetworkFetchPolicy ? NetworkStatus.loading :
+      NetworkStatus.ready;
+
+    const result: ApolloCurrentQueryResult<TData> = {
+      data: !lastError && lastResult && lastResult.data || void 0,
+      error: this.lastError,
+      loading: isNetworkRequestInFlight(networkStatus),
+      networkStatus,
+      stale: lastResult ? lastResult.stale : false,
+    };
+
+    if (this.isTornDown) {
+      return result;
+    }
+
+    const queryStoreValue = this.queryManager.queryStore.get(this.queryId);
     if (queryStoreValue) {
       const { networkStatus } = queryStoreValue;
 
       if (hasError(queryStoreValue, this.options.errorPolicy)) {
-        return {
+        return Object.assign(result, {
           data: void 0,
-          loading: false,
           networkStatus,
           error: new ApolloError({
             graphQLErrors: queryStoreValue.graphQLErrors,
             networkError: queryStoreValue.networkError,
           }),
-        };
+        });
       }
 
       // Variables might have been added dynamically at query time, when
@@ -192,38 +183,19 @@ export class ObservableQuery<
         this.variables = this.options.variables;
       }
 
-      result = {
-        data,
+      Object.assign(result, {
         loading: isNetworkRequestInFlight(networkStatus),
         networkStatus,
-      } as ApolloQueryResult<TData>;
+      });
 
       if (queryStoreValue.graphQLErrors && this.options.errorPolicy === 'all') {
         result.errors = queryStoreValue.graphQLErrors;
       }
-
-    } else {
-      // We need to be careful about the loading state we show to the user, to try
-      // and be vaguely in line with what the user would have seen from .subscribe()
-      // but to still provide useful information synchronously when the query
-      // will not end up hitting the server.
-      // See more: https://github.com/apollostack/apollo-client/issues/707
-      // Basically: is there a query in flight right now (modolo the next tick)?
-      const loading = isNetworkFetchPolicy ||
-        (partial && fetchPolicy !== 'cache-only');
-
-      result = {
-        data,
-        loading,
-        networkStatus: loading ? NetworkStatus.loading : NetworkStatus.ready,
-      } as ApolloQueryResult<TData>;
     }
 
-    if (!partial) {
-      this.updateLastResult({ ...result, stale: false });
-    }
+    this.updateLastResult(result);
 
-    return { ...result, partial };
+    return result;
   }
 
   // Compares newResult to the snapshot we took of this.lastResult when it was
@@ -545,6 +517,9 @@ export class ObservableQuery<
     this.lastResultSnapshot = this.queryManager.assumeImmutableResults
       ? newResult
       : cloneDeep(newResult);
+    if (!isNonEmptyArray(newResult.errors)) {
+      delete this.lastError;
+    }
     return previousResult;
   }
 

diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
@@ -431,9 +431,9 @@ export class QueryManager<TStore> {
       fetchMoreForQueryId,
     });
 
-    this.broadcastQueries();
-
     if (shouldFetch) {
+      this.broadcastQueries();
+
       const networkResult = this.fetchRequest<T>({
         requestId,
         queryId,
@@ -765,11 +765,24 @@ export class QueryManager<TStore> {
 
     let transformedOptions = { ...options } as WatchQueryOptions<TVariables>;
 
-    return new ObservableQuery<T, TVariables>({
+    const observable = new ObservableQuery<T, TVariables>({
       queryManager: this,
       options: transformedOptions,
       shouldSubscribe: shouldSubscribe,
     });
+
+    this.queryStore.initQuery({
+      queryId: observable.queryId,
+      document: this.transform(options.query).document,
+      variables: options.variables,
+      storePreviousVariables: false,
+      isPoll: typeof options.pollInterval === 'number',
+      isRefetch: false,
+      metadata: options.metadata,
+      fetchMoreForQueryId: void 0,
+    });
+
+    return observable;
   }
 
   public query<T>(options: QueryOptions): Promise<ApolloQueryResult<T>> {
@@ -1034,7 +1047,7 @@ export class QueryManager<TStore> {
     this.queries.delete(queryId);
   }
 
-  public getCurrentQueryResult<T>(
+  private getCurrentQueryResult<T>(
     observableQuery: ObservableQuery<T>,
     optimistic: boolean = true,
   ): {