Split FetchPolicy type to forbid passing cache-and-network to client.…

…query.

#3130 (comment)

@PowerKiKi How does this look to you?

packages/apollo-client/src/ApolloClient.ts

@@ -20,24 +20,21 @@ import { LocalState, FragmentMatcher } from './core/LocalState';
 import { Observable } from './util/Observable';
 
 import {
-  QueryBaseOptions,
   QueryOptions,
   WatchQueryOptions,
   SubscriptionOptions,
   MutationOptions,
-  ModifiableWatchQueryOptions,
-  MutationBaseOptions,
+  WatchQueryFetchPolicy,
 } from './core/watchQueryOptions';
 
 import { DataStore } from './data/store';
 
 import { version } from './version';
 
-
 export interface DefaultOptions {
-  watchQuery?: ModifiableWatchQueryOptions;
-  query?: QueryBaseOptions;
-  mutate?: MutationBaseOptions;
+  watchQuery?: Partial<WatchQueryOptions>;
+  query?: Partial<QueryOptions>;
+  mutate?: Partial<MutationOptions>;
 }
 
 let hasSuggestedDevtools = false;
@@ -316,24 +313,18 @@ export default class ApolloClient<TCacheShape> implements DataProxy {
       >;
     }
 
-    const accidentallyCacheAndNetwork =
-      options.fetchPolicy === 'cache-and-network';
+    invariant(
+      (options.fetchPolicy as WatchQueryFetchPolicy) !== 'cache-and-network',
+      'The cache-and-network fetchPolicy does not work with client.query, because ' +
+      'client.query can only return a single result. Please use client.watchQuery ' +
+      'to receive multiple results from the cache and the network, or consider ' +
+      'using a different fetchPolicy, such as cache-first or network-only.'
+    );
 
-    if (
-      accidentallyCacheAndNetwork ||
-      (this.disableNetworkFetches && options.fetchPolicy === 'network-only')
-    ) {
+    if (this.disableNetworkFetches && options.fetchPolicy === 'network-only') {
       options = { ...options, fetchPolicy: 'cache-first' };
     }
 
-    if (accidentallyCacheAndNetwork) {
-      invariant.warn(
-        'The cache-and-network fetchPolicy has been converted to cache-first, ' +
-        'since client.query can only return a single result. Please use watchQuery ' +
-        'instead to receive multiple results from the cache and the network.'
-      );
-    }
-
     return this.queryManager.query<T>(options);
   }
 

packages/apollo-client/src/__tests__/client.ts

@@ -10,7 +10,7 @@ import {
 import { stripSymbols } from 'apollo-utilities';
 
 import { QueryManager } from '../core/QueryManager';
-import { WatchQueryOptions } from '../core/watchQueryOptions';
+import { WatchQueryOptions, FetchPolicy } from '../core/watchQueryOptions';
 
 import { ApolloError } from '../errors/ApolloError';
 
@@ -1725,32 +1725,33 @@ describe('client', () => {
       },
     };
 
-    function checkWarning(callback: () => any, message: string) {
-      const { warn } = console;
-      const messages: string[] = [];
-      console.warn = (message: string) => messages.push(message);
+    const cacheAndNetworkError =
+      'The cache-and-network fetchPolicy does not work with client.query, because ' +
+      'client.query can only return a single result. Please use client.watchQuery ' +
+      'to receive multiple results from the cache and the network, or consider ' +
+      'using a different fetchPolicy, such as cache-first or network-only.';
+
+    function checkCacheAndNetworkError(callback: () => any) {
       try {
         callback();
-      } finally {
-        console.warn = warn;
+        throw new Error('not reached');
+      } catch (thrown) {
+        expect(thrown.message).toBe(cacheAndNetworkError);
       }
-      expect(messages).toContain(message);
     }
 
-    const cacheAndNetworkWarning =
-      'The cache-and-network fetchPolicy has been converted to cache-first, ' +
-      'since client.query can only return a single result. Please use watchQuery ' +
-      'instead to receive multiple results from the cache and the network.';
-
     // Test that cache-and-network can only be used on watchQuery, not query.
     it('warns when used with client.query', () => {
       const client = new ApolloClient({
         link: ApolloLink.empty(),
         cache: new InMemoryCache(),
       });
-      checkWarning(
-        () => client.query({ query, fetchPolicy: 'cache-and-network' }),
-        cacheAndNetworkWarning,
+
+      checkCacheAndNetworkError(
+        () => client.query({
+          query,
+          fetchPolicy: 'cache-and-network' as FetchPolicy,
+        }),
       );
     });
 
@@ -1760,11 +1761,12 @@ describe('client', () => {
         cache: new InMemoryCache(),
         defaultOptions: {
           query: {
-            fetchPolicy: 'cache-and-network',
+            fetchPolicy: 'cache-and-network' as FetchPolicy,
           },
         },
       });
-      checkWarning(() => client.query({ query }), cacheAndNetworkWarning);
+
+      checkCacheAndNetworkError(() => client.query({ query }));
     });
 
     it('fetches from cache first, then network', done => {

packages/apollo-client/src/core/watchQueryOptions.ts

@@ -17,12 +17,13 @@ import { PureQueryOptions, OperationVariables } from './types';
  */
 export type FetchPolicy =
   | 'cache-first'
-  | 'cache-and-network'
   | 'network-only'
   | 'cache-only'
   | 'no-cache'
   | 'standby';
 
+export type WatchQueryFetchPolicy = FetchPolicy | 'cache-and-network';
+
 /**
  * errorPolicy determines the level of events for errors in the execution result. The options are:
  * - none (default): any errors from the request are treated like runtime errors and the observable is stopped (XXX this is default to lower breaking changes going from AC 1.0 => 2.0)
@@ -36,15 +37,18 @@ export type ErrorPolicy = 'none' | 'ignore' | 'all';
  */
 export interface QueryBaseOptions<TVariables = OperationVariables> {
   /**
-   * A map going from variable name to variable value, where the variables are used
-   * within the GraphQL query.
+   * A GraphQL document that consists of a single query to be sent down to the
+   * server.
    */
-  variables?: TVariables;
+  // TODO REFACTOR: rename this to document. Didn't do it yet because it's in a
+  // lot of tests.
+  query: DocumentNode;
 
   /**
-   * Specifies the {@link FetchPolicy} to be used for this query
+   * A map going from variable name to variable value, where the variables are used
+   * within the GraphQL query.
    */
-  fetchPolicy?: FetchPolicy;
+  variables?: TVariables;
 
   /**
    * Specifies the {@link ErrorPolicy} to be used for this query
@@ -55,20 +59,6 @@ export interface QueryBaseOptions<TVariables = OperationVariables> {
    * Whether or not to fetch results
    */
   fetchResults?: boolean;
-}
-
-/**
- * Query options.
- */
-export interface QueryOptions<TVariables = OperationVariables>
-  extends QueryBaseOptions<TVariables> {
-  /**
-   * A GraphQL document that consists of a single query to be sent down to the
-   * server.
-   */
-  // TODO REFACTOR: rename this to document. Didn't do it yet because it's in a
-  // lot of tests.
-  query: DocumentNode;
 
   /**
    * Arbitrary metadata stored in the store with this query.  Designed for debugging,
@@ -82,6 +72,17 @@ export interface QueryOptions<TVariables = OperationVariables>
   context?: any;
 }
 
+/**
+ * Query options.
+ */
+export interface QueryOptions<TVariables = OperationVariables>
+  extends QueryBaseOptions<TVariables> {
+  /**
+   * Specifies the {@link FetchPolicy} to be used for this query
+   */
+  fetchPolicy?: FetchPolicy;
+}
+
 /**
  * We can change these options to an ObservableQuery
  */
@@ -109,8 +110,13 @@ export interface ModifiableWatchQueryOptions<TVariables = OperationVariables>
  * Watched query options.
  */
 export interface WatchQueryOptions<TVariables = OperationVariables>
-  extends QueryOptions<TVariables>,
-    ModifiableWatchQueryOptions<TVariables> {}
+  extends QueryBaseOptions<TVariables>,
+    ModifiableWatchQueryOptions<TVariables> {
+  /**
+   * Specifies the {@link FetchPolicy} to be used for this query
+   */
+  fetchPolicy?: WatchQueryFetchPolicy;
+}
 
 export interface FetchMoreQueryOptions<TVariables, K extends keyof TVariables> {
   query?: DocumentNode;