Skip to content

Commit

Permalink
Merge pull request #1371 from apollographql/fetch-policy
Browse files Browse the repository at this point in the history
WIP: replace noFetch and forceFetch with fetch policy
  • Loading branch information
helfer authored Mar 9, 2017
2 parents 8c94f1b + d51fbc5 commit d420cbd
Show file tree
Hide file tree
Showing 17 changed files with 239 additions and 150 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Expect active development and potentially significant breaking changes in the `0
- Fix: make sure maybeDeepFreeze is called on results returned from setVariables and refetch [PR #1362](https://github.com/apollographql/apollo-client/pull/1362)
- Fix: use setTimeout to throw uncaught errors in observer.next and observer.error[PR #1367](https://github.com/apollographql/apollo-client/pull/1367)
- Remove returnPartialData option [PR #1370](https://github.com/apollographql/apollo-client/pull/1370)
- Implement fetchPolicy to replace noFetch and forceFetch [PR #1371](https://github.com/apollographql/apollo-client/pull/1371)


### 0.10.1
Expand Down
6 changes: 3 additions & 3 deletions benchmark/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ group((end) => {
const client = getClientInstance();
const observable = client.watchQuery({
query: simpleQuery,
noFetch: true,
fetchPolicy: 'cache-only',
});
observable.subscribe({
next(res: ApolloQueryResult<Object>) {
Expand Down Expand Up @@ -145,7 +145,7 @@ group((end) => {
promises.push(new Promise<void>((resolve, reject) => {
client.watchQuery({
query: simpleQuery,
noFetch: true,
fetchPolicy: 'cache-only',
}).subscribe({
next(res: ApolloQueryResult<Object>) {
if (Object.keys(res.data).length > 0) {
Expand Down Expand Up @@ -291,7 +291,7 @@ times(50, (index) => {
client.query({
query,
variables,
noFetch: true,
fetchPolicy: 'cache-only',
}).then(() => {
done();
});
Expand Down
21 changes: 12 additions & 9 deletions src/ApolloClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export default class ApolloClient implements DataProxy {
public queryManager: QueryManager;
public reducerConfig: ApolloReducerConfig;
public addTypename: boolean;
public shouldForceFetch: boolean;
public disableNetworkFetches: boolean;
public dataId: IdGetter | undefined;
public fieldWithArgs: (fieldName: string, args?: Object) => string;
public version: string;
Expand Down Expand Up @@ -175,13 +175,13 @@ export default class ApolloClient implements DataProxy {
this.networkInterface = networkInterface ? networkInterface :
createNetworkInterface({ uri: '/graphql' });
this.addTypename = addTypename;
this.shouldForceFetch = !(ssrMode || ssrForceFetchDelay > 0);
this.disableNetworkFetches = ssrMode || ssrForceFetchDelay > 0;
this.dataId = dataIdFromObject;
this.fieldWithArgs = storeKeyNameFromFieldNameAndArgs;
this.queryDeduplication = queryDeduplication;

if (ssrForceFetchDelay) {
setTimeout(() => this.shouldForceFetch = true, ssrForceFetchDelay);
setTimeout(() => this.disableNetworkFetches = false, ssrForceFetchDelay);
}

this.reducerConfig = {
Expand Down Expand Up @@ -229,10 +229,11 @@ export default class ApolloClient implements DataProxy {
public watchQuery<T>(options: WatchQueryOptions): ObservableQuery<T> {
this.initStore();

if (!this.shouldForceFetch && options.forceFetch) {
// XXX Overwriting options is probably not the best way to do this long term...
if (this.disableNetworkFetches && options.fetchPolicy === 'network-only') {
options = {
...options,
forceFetch: false,
fetchPolicy: 'cache-first',
} as WatchQueryOptions;
}

Expand All @@ -251,13 +252,15 @@ export default class ApolloClient implements DataProxy {
public query<T>(options: WatchQueryOptions): Promise<ApolloQueryResult<T>> {
this.initStore();

// XXX what if I pass pollInterval? Will it just keep running?
// XXX why doesn't this stop the query after it's done?
if (options.fetchPolicy === 'cache-and-network') {
throw new Error('cache-and-network fetchPolicy can only be used with watchQuery');
}

if (!this.shouldForceFetch && options.forceFetch) {
// XXX Overwriting options is probably not the best way to do this long term...
if (this.disableNetworkFetches && options.fetchPolicy === 'network-only') {
options = {
...options,
forceFetch: false,
fetchPolicy: 'cache-first',
} as WatchQueryOptions;
}

Expand Down
6 changes: 5 additions & 1 deletion src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ import {
ApolloReducer,
} from './store';

import {
FetchPolicy,
} from './core/watchQueryOptions';

export type QueryResultAction = {
type: 'APOLLO_QUERY_RESULT';
result: ExecutionResult;
Expand Down Expand Up @@ -47,7 +51,7 @@ export interface QueryInitAction {
queryString: string;
document: DocumentNode;
variables: Object;
forceFetch: boolean;
fetchPolicy: FetchPolicy;
queryId: string;
requestId: number;
storePreviousVariables: boolean;
Expand Down
31 changes: 16 additions & 15 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ export class ObservableQuery<T> extends Observable<ApolloQueryResult<T>> {
// 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 = (this.options.forceFetch && queryLoading)
|| (partial && !this.options.noFetch);
const loading = (this.options.fetchPolicy === 'network-only' && queryLoading)
|| (partial && this.options.fetchPolicy !== 'cache-only');

// if there is nothing in the query store, it means this query hasn't fired yet. Therefore the
// network status is dependent on queryLoading.
Expand Down Expand Up @@ -180,8 +180,8 @@ export class ObservableQuery<T> extends Observable<ApolloQueryResult<T>> {
...variables,
};

if (this.options.noFetch) {
throw new Error('noFetch option should not use query refetch.');
if (this.options.fetchPolicy === 'cache-only') {
throw new Error('cache-only fetchPolicy option should not be used together with query refetch.');
}

// Update the existing options with new variables
Expand All @@ -190,10 +190,10 @@ export class ObservableQuery<T> extends Observable<ApolloQueryResult<T>> {
...this.variables,
};

// Override forceFetch for this call only
const combinedOptions = {
// Override fetchPolicy for this call only
const combinedOptions: WatchQueryOptions = {
...this.options,
forceFetch: true,
fetchPolicy: 'network-only',
};

return this.queryManager.fetchQuery(this.queryId, combinedOptions, FetchType.refetch)
Expand Down Expand Up @@ -228,7 +228,7 @@ export class ObservableQuery<T> extends Observable<ApolloQueryResult<T>> {
combinedOptions = {
...combinedOptions,
query: combinedOptions.query,
forceFetch: true,
fetchPolicy: 'network-only',
} as WatchQueryOptions;
return this.queryManager.fetchQuery(qid, combinedOptions, FetchType.normal, this.queryId);
})
Expand Down Expand Up @@ -310,9 +310,10 @@ export class ObservableQuery<T> extends Observable<ApolloQueryResult<T>> {
this.stopPolling();
}

// If forceFetch went from false to true or noFetch went from true to false
const tryFetch: boolean = (!oldOptions.forceFetch && opts.forceFetch)
|| (oldOptions.noFetch && !opts.noFetch) || false;
// If fetchPolicy went from cache-only to something else, or from something else to network-only
const tryFetch: boolean = (oldOptions.fetchPolicy !== 'network-only' && opts.fetchPolicy === 'network-only')
|| (oldOptions.fetchPolicy === 'cache-only' && opts.fetchPolicy !== 'cache-only')
|| false;

return this.setVariables(this.options.variables, tryFetch);
}
Expand Down Expand Up @@ -397,8 +398,8 @@ export class ObservableQuery<T> extends Observable<ApolloQueryResult<T>> {
}

public startPolling(pollInterval: number) {
if (this.options.noFetch) {
throw new Error('noFetch option should not use query polling.');
if (this.options.fetchPolicy === 'cache-first' || (this.options.fetchPolicy === 'cache-only')) {
throw new Error('Queries that specify the cache-first and cache-only fetchPolicies cannot also be polling queries.');
}

if (this.isCurrentlyPolling) {
Expand Down Expand Up @@ -450,8 +451,8 @@ export class ObservableQuery<T> extends Observable<ApolloQueryResult<T>> {
}

if (!!this.options.pollInterval) {
if (this.options.noFetch) {
throw new Error('noFetch option should not use query polling.');
if (this.options.fetchPolicy === 'cache-first' || (this.options.fetchPolicy === 'cache-only')) {
throw new Error('Queries that specify the cache-first and cache-only fetchPolicies cannot also be polling queries.');
}

this.isCurrentlyPolling = true;
Expand Down
40 changes: 24 additions & 16 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ export class QueryManager {
this.query({
query: pureQuery.query,
variables: pureQuery.variables,
forceFetch: true,
fetchPolicy: 'network-only',
});
});
}
Expand Down Expand Up @@ -341,9 +341,12 @@ export class QueryManager {
return;
}

const noFetch = this.observableQueries[queryId] ? this.observableQueries[queryId].observableQuery.options.noFetch : options.noFetch;
const storedQuery = this.observableQueries[queryId];

const shouldNotifyIfLoading = queryStoreValue.previousVariables || noFetch;
const fetchPolicy = storedQuery ? storedQuery.observableQuery.options.fetchPolicy : options.fetchPolicy;

const shouldNotifyIfLoading = queryStoreValue.previousVariables ||
fetchPolicy === 'cache-only' || fetchPolicy === 'cache-and-network';

const networkStatusChanged = lastResult && queryStoreValue.networkStatus !== lastResult.networkStatus;

Expand Down Expand Up @@ -396,7 +399,7 @@ export class QueryManager {
// If there is some data missing and the user has told us that they
// do not tolerate partial data then we want to return the previous
// result and mark it as stale.
if (isMissing && !noFetch) {
if (isMissing && fetchPolicy !== 'cache-only') {
resultFromStore = {
data: lastResult && lastResult.data,
loading: isNetworkRequestInFlight(queryStoreValue.networkStatus),
Expand Down Expand Up @@ -521,11 +524,11 @@ export class QueryManager {
// network status for the query this is fetching for.
fetchMoreForQueryId?: string,
): Promise<ApolloQueryResult<T>> {

const {
variables = {},
forceFetch = false,
noFetch = false,
metadata = null,
fetchPolicy = 'cache-first', // cache-first is the default fetch policy.
} = options;

const {
Expand All @@ -535,11 +538,12 @@ export class QueryManager {
const queryString = print(queryDoc);

let storeResult: any;
let needToFetch: boolean = forceFetch;
let needToFetch: boolean = fetchPolicy === 'network-only';

// If this is not a force fetch, we want to diff the query against the
// store before we fetch it from the network interface.
if (fetchType !== FetchType.refetch && !forceFetch) {
// TODO we hit the cache even if the policy is network-first. This could be unnecessary if the network is up.
if ( (fetchType !== FetchType.refetch && fetchPolicy !== 'network-only')) {
const { isMissing, result } = diffQueryAgainstStore({
query: queryDoc,
store: this.reduxRootSelector(this.store.getState()).data,
Expand All @@ -549,13 +553,13 @@ export class QueryManager {
});

// If we're in here, only fetch if we have missing fields
needToFetch = isMissing || false;
needToFetch = isMissing || fetchPolicy === 'cache-and-network';

storeResult = result;
}

const requestId = this.generateRequestId();
const shouldFetch = needToFetch && !noFetch;
const shouldFetch = needToFetch && fetchPolicy !== 'cache-only';

// Initialize query in store with unique requestId
this.queryDocuments[queryId] = queryDoc;
Expand All @@ -564,7 +568,7 @@ export class QueryManager {
queryString,
document: queryDoc,
variables,
forceFetch,
fetchPolicy,
queryId,
requestId,
// we store the old variables in order to trigger "loading new variables"
Expand All @@ -577,7 +581,7 @@ export class QueryManager {
});

// If there is no part of the query we need to fetch from the server (or,
// noFetch is turned on), we just write the store result as the final result.
// cachePolicy is cache-only), we just write the store result as the final result.
if (!shouldFetch) {
this.store.dispatch({
type: 'APOLLO_QUERY_RESULT_CLIENT',
Expand All @@ -591,15 +595,18 @@ export class QueryManager {
}

if (shouldFetch) {
return this.fetchRequest({
const networkResult = this.fetchRequest({
requestId,
queryId,
document: queryDoc,
options,
fetchMoreForQueryId,
});
}

if (fetchPolicy !== 'cache-and-network') {
return networkResult;
}
}
// If we have no query to send to the server, we should return the result
// found within the store.
return Promise.resolve({ data: storeResult });
Expand Down Expand Up @@ -704,7 +711,9 @@ export class QueryManager {
Object.keys(this.observableQueries).forEach((queryId) => {
const storeQuery = this.reduxRootSelector(this.store.getState()).queries[queryId];

if (!this.observableQueries[queryId].observableQuery.options.noFetch) {
const fetchPolicy = this.observableQueries[queryId].observableQuery.options.fetchPolicy;

if (fetchPolicy !== 'cache-only') {
this.observableQueries[queryId].observableQuery.refetch();
}
});
Expand Down Expand Up @@ -927,7 +936,6 @@ export class QueryManager {
}): Promise<ExecutionResult> {
const {
variables,
noFetch,
} = options;
const request: Request = {
query: document,
Expand Down
32 changes: 12 additions & 20 deletions src/core/watchQueryOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ import {
PureQueryOptions,
} from './types';

/**
* fetchPolicy determines where the client may return a result from. The options are:
* - cache-first (default): return result from cache. Only fetch from network if cached result is not available.
* - cache-and-network: returns result from cache first (if it exists), then return network result once it's available
* - cache-only: return result from cache if avaiable, fail otherwise.
* - network-only: return result from network, fail if network call doesn't succeed.
*/

export type FetchPolicy = 'cache-first' | 'cache-and-network' | 'network-only' | 'cache-only';

/**
* We can change these options to an ObservableQuery
*/
Expand All @@ -25,35 +35,17 @@ export interface ModifiableWatchQueryOptions {
* within the GraphQL query.
*/
variables?: { [key: string]: any };
/**
* Specifies whether the client should diff the query against the cache and only
* fetch the portions of it that aren't already available (it does this when forceFetch is
* false) or it should just fetch the entire query from the server and update the cache
* accordingly (it does this when forceFetch is true).
*/
forceFetch?: boolean;

/**
* If this is set to true, the query is resolved *only* within information
* available in the cache (i.e. we never hit the server). If a particular field is not available
* in the cache, it will not be available in the result.
*/
noFetch?: boolean;
/**
* The time interval (in milliseconds) on which this query should be
* refetched from the server.
*/
pollInterval?: number;

/**
* fetchPolicy determines where the client may return a result from. The options are:
* - cache-and-network: returns result from cache first (if it exists), then return network result once it's available
* - cache-first: return result from cache. Only fetch from network if cached result is not available.
* - network-first: return result from network, but if network request fails, use result from cache, if available.
* - cache-only: return result from cache if avaiable, fail otherwise.
* - network-only: return result from network, fail if network call doesn't succeed.
* Specifies the {@link FetchPolicy} to be used for this query
*/
// fetchPolicy?: string;
fetchPolicy?: FetchPolicy;

/**
* Whether or not updates to the network status should trigger next on the observer of this query
Expand Down
Loading

0 comments on commit d420cbd

Please sign in to comment.