From da9fd7bc5482cd35e839c92033a8750fbd4cc7d7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 1 May 2019 14:19:18 -0400 Subject: [PATCH] Split FetchPolicy type to forbid passing cache-and-network to client.query. https://github.com/apollographql/apollo-client/issues/3130#issuecomment-478409066 @PowerKiKi How does this look to you? --- packages/apollo-client/src/ApolloClient.ts | 33 +++++------- .../apollo-client/src/__tests__/client.ts | 38 +++++++------- .../src/core/watchQueryOptions.ts | 50 +++++++++++-------- 3 files changed, 60 insertions(+), 61 deletions(-) diff --git a/packages/apollo-client/src/ApolloClient.ts b/packages/apollo-client/src/ApolloClient.ts index 062e3ddb6b0..c409d05ef04 100644 --- a/packages/apollo-client/src/ApolloClient.ts +++ b/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; + query?: Partial; + mutate?: Partial; } let hasSuggestedDevtools = false; @@ -316,24 +313,18 @@ export default class ApolloClient 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(options); } diff --git a/packages/apollo-client/src/__tests__/client.ts b/packages/apollo-client/src/__tests__/client.ts index 1930535475f..3f849aae3c2 100644 --- a/packages/apollo-client/src/__tests__/client.ts +++ b/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 => { diff --git a/packages/apollo-client/src/core/watchQueryOptions.ts b/packages/apollo-client/src/core/watchQueryOptions.ts index 238fdb4aae2..809157cb7d7 100644 --- a/packages/apollo-client/src/core/watchQueryOptions.ts +++ b/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 { /** - * 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 { * Whether or not to fetch results */ fetchResults?: boolean; -} - -/** - * Query options. - */ -export interface QueryOptions - extends QueryBaseOptions { - /** - * 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 context?: any; } +/** + * Query options. + */ +export interface QueryOptions + extends QueryBaseOptions { + /** + * 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 * Watched query options. */ export interface WatchQueryOptions - extends QueryOptions, - ModifiableWatchQueryOptions {} + extends QueryBaseOptions, + ModifiableWatchQueryOptions { + /** + * Specifies the {@link FetchPolicy} to be used for this query + */ + fetchPolicy?: WatchQueryFetchPolicy; +} export interface FetchMoreQueryOptions { query?: DocumentNode;