From b8ee3a6b3d0e63aafffa5e440f1ed70e0f5f4352 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 9 Mar 2021 12:06:48 -0800 Subject: [PATCH 1/9] add public method to retrieve all current observable queries --- src/__tests__/client.ts | 12 ++++++++++++ src/core/ApolloClient.ts | 8 ++++++++ src/core/QueryManager.ts | 32 ++++++++++++++++++++------------ 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 446693a53d9..9e0b4e40f9b 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -2442,6 +2442,18 @@ describe('client', () => { expect(spy).toHaveBeenCalledWith(options); }); + it('has a getObservableQueries method which calls QueryManager', async () => { + const client = new ApolloClient({ + link: ApolloLink.empty(), + cache: new InMemoryCache(), + }); + + // @ts-ignore + const spy = jest.spyOn(client.queryManager, 'getObservableQueries'); + await client.getObservableQueries(); + expect(spy).toHaveBeenCalled(); + }); + itAsync('should propagate errors from network interface to observers', (resolve, reject) => { const link = ApolloLink.from([ () => diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index 53485777b07..2d902549e43 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -603,6 +603,14 @@ export class ApolloClient implements DataProxy { return this.localState.getResolvers(); } + /** + * Get all observable queries which have current observers (e.g. + * they're mounted). + */ + public getObservableQueries() { + return this.queryManager.getObservableQueries(); + } + /** * Set a custom local state fragment matcher. */ diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index cb08575deb5..9f273256e40 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -734,25 +734,33 @@ export class QueryManager { }); } + public getObservableQueries() { + const queries = new Map>(); + this.queries.forEach(({ observableQuery }, queryId) => { + if (observableQuery && observableQuery.hasObservers()) { + queries.set(queryId, observableQuery); + } + }); + return queries; + } + public reFetchObservableQueries( includeStandby: boolean = false, ): Promise[]> { const observableQueryPromises: Promise>[] = []; - this.queries.forEach(({ observableQuery }, queryId) => { - if (observableQuery && observableQuery.hasObservers()) { - const fetchPolicy = observableQuery.options.fetchPolicy; - - observableQuery.resetLastResults(); - if ( - fetchPolicy !== 'cache-only' && - (includeStandby || fetchPolicy !== 'standby') - ) { - observableQueryPromises.push(observableQuery.refetch()); - } + this.getObservableQueries().forEach((observableQuery, queryId) => { + const fetchPolicy = observableQuery.options.fetchPolicy; - this.getQuery(queryId).setDiff(null); + observableQuery.resetLastResults(); + if ( + fetchPolicy !== 'cache-only' && + (includeStandby || fetchPolicy !== 'standby') + ) { + observableQueryPromises.push(observableQuery.refetch()); } + + this.getQuery(queryId).setDiff(null); }); this.broadcastQueries(); From e673f8b78e30bb2fd78c3bc66fb2ee8a487ac5b9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 2 Jun 2021 16:28:29 -0400 Subject: [PATCH 2/9] Use getObservableQueries for refetchQueries include handling. --- src/core/ApolloClient.ts | 24 +++-- src/core/QueryManager.ts | 191 +++++++++++++++++++-------------------- src/core/types.ts | 2 +- 3 files changed, 112 insertions(+), 105 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index 2d902549e43..58297858ca0 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -18,6 +18,7 @@ import { RefetchQueriesOptions, RefetchQueriesResult, InternalRefetchQueriesResult, + RefetchQueryDescription, } from './types'; import { @@ -564,6 +565,21 @@ export class ApolloClient implements DataProxy { return result; } + /** + * Get all currently active ObservableQuery objects, in a Map keyed by query + * ID strings. An "active" query is one that has observers and a fetchPolicy + * other than "standby" or "cache-only". You can include all ObservableQuery + * objects (including the inactive ones) by passing "all" instead of "active", + * or you can include just a subset of active queries by passing an array of + * query names or DocumentNode objects. This method is used internally by + * `client.refetchQueries` to handle its `include` option. + */ + public getObservableQueries( + include: RefetchQueryDescription = "active", + ) { + return this.queryManager.getObservableQueries(include); + } + /** * Exposes the cache's complete state, in a serializable format for later restoration. */ @@ -603,14 +619,6 @@ export class ApolloClient implements DataProxy { return this.localState.getResolvers(); } - /** - * Get all observable queries which have current observers (e.g. - * they're mounted). - */ - public getObservableQueries() { - return this.queryManager.getObservableQueries(); - } - /** * Set a custom local state fragment matcher. */ diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 9f273256e40..9933501f43a 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -41,7 +41,6 @@ import { OnQueryUpdated, RefetchQueryDescription, InternalRefetchQueriesOptions, - RefetchQueryDescriptor, InternalRefetchQueriesResult, InternalRefetchQueriesMap, } from './types'; @@ -734,13 +733,73 @@ export class QueryManager { }); } - public getObservableQueries() { + public getObservableQueries( + include: RefetchQueryDescription = "active", + ) { const queries = new Map>(); - this.queries.forEach(({ observableQuery }, queryId) => { - if (observableQuery && observableQuery.hasObservers()) { - queries.set(queryId, observableQuery); + const queryNames = new Set(); + const queryDocs = new Set(); + const legacyQueryOptions = new Set(); + + if (Array.isArray(include)) { + include.forEach(desc => { + if (typeof desc === "string") { + queryNames.add(desc); + } else if (isDocumentNode(desc)) { + queryDocs.add(desc); + } else if (isNonNullObject(desc) && desc.query) { + legacyQueryOptions.add(desc); + } + }); + } + + this.queries.forEach(({ observableQuery: oq, document }, queryId) => { + if (oq) { + if (include === "all") { + queries.set(queryId, oq); + return; + } + + const { fetchPolicy } = oq.options; + if (fetchPolicy === "cache-only" || + fetchPolicy === "standby" || + !oq.hasObservers()) { + // Skip inactive queries unless include === "all". + return; + } + + if ( + include === "active" || + queryNames.has(oq.queryName!) || + (document && queryDocs.has(document)) + ) { + queries.set(queryId, oq); + } } }); + + if (legacyQueryOptions.size) { + legacyQueryOptions.forEach((options: QueryOptions) => { + // We will be issuing a fresh network request for this query, so we + // pre-allocate a new query ID here. + const queryId = this.generateQueryId(); + const queryInfo = this.getQuery(queryId).init({ + document: options.query, + variables: options.variables, + }); + const oq = new ObservableQuery({ + queryManager: this, + queryInfo, + options: { + ...options, + fetchPolicy: "network-only", + }, + }); + queryInfo.setObservableQuery(oq); + queries.set(queryId, oq); + }); + } + return queries; } @@ -749,17 +808,11 @@ export class QueryManager { ): Promise[]> { const observableQueryPromises: Promise>[] = []; - this.getObservableQueries().forEach((observableQuery, queryId) => { - const fetchPolicy = observableQuery.options.fetchPolicy; - + this.getObservableQueries( + includeStandby ? "all" : "active" + ).forEach((observableQuery, queryId) => { observableQuery.resetLastResults(); - if ( - fetchPolicy !== 'cache-only' && - (includeStandby || fetchPolicy !== 'standby') - ) { - observableQueryPromises.push(observableQuery.refetch()); - } - + observableQueryPromises.push(observableQuery.refetch()); this.getQuery(queryId).setDiff(null); }); @@ -1112,21 +1165,17 @@ export class QueryManager { onQueryUpdated, }: InternalRefetchQueriesOptions, TResult> ): InternalRefetchQueriesMap { - const includedQueriesById = new Map; lastDiff?: Cache.DiffResult; diff?: Cache.DiffResult; }>(); if (include) { - include.forEach(desc => { - getQueryIdsForQueryDescriptor(this, desc).forEach(queryId => { - includedQueriesById.set(queryId, { - desc, - lastDiff: typeof desc === "string" || isDocumentNode(desc) - ? this.getQuery(queryId).getDiff() - : void 0, - }); + this.getObservableQueries(include).forEach((oq, queryId) => { + includedQueries.set(queryId, { + oq, + lastDiff: this.getQuery(queryId).getDiff(), }); }); } @@ -1187,7 +1236,7 @@ export class QueryManager { // Since we're about to handle this query now, remove it from // includedQueriesById, in case it was added earlier because of // options.include. - includedQueriesById.delete(oq.queryId); + includedQueries.delete(oq.queryId); let result: boolean | InternalRefetchQueriesResult = onQueryUpdated(oq, diff, lastDiff); @@ -1213,58 +1262,35 @@ export class QueryManager { // If we don't have an onQueryUpdated function, and onQueryUpdated // was not disabled by passing null, make sure this query is // "included" like any other options.include-specified query. - includedQueriesById.set(oq.queryId, { - desc: oq.queryName || ``, - lastDiff, - diff, - }); + includedQueries.set(oq.queryId, { oq, lastDiff, diff }); } } }, }); } - if (includedQueriesById.size) { - includedQueriesById.forEach(({ desc, lastDiff, diff }, queryId) => { - const queryInfo = this.getQuery(queryId); - let oq = queryInfo.observableQuery; - let fallback: undefined | (() => Promise>); - - if (typeof desc === "string" || isDocumentNode(desc)) { - fallback = () => oq!.refetch(); - } else if (isNonNullObject(desc)) { - const options = { - ...desc, - fetchPolicy: "network-only", - } as QueryOptions; - - queryInfo.setObservableQuery(oq = new ObservableQuery({ - queryManager: this, - queryInfo, - options, - })); + if (includedQueries.size) { + includedQueries.forEach(({ oq, lastDiff, diff }, queryId) => { + let result: undefined | boolean | InternalRefetchQueriesResult; + + // If onQueryUpdated is provided, we want to use it for all included + // queries, even the PureQueryOptions ones. + if (onQueryUpdated) { + if (!diff) { + const info = oq["queryInfo"]; + info.reset(); // Force info.getDiff() to read from cache. + diff = info.getDiff(); + } + result = onQueryUpdated(oq, diff, lastDiff); + } - fallback = () => this.query(options, queryId); + // Otherwise, we fall back to refetching. + if (!onQueryUpdated || result === true) { + result = oq.refetch(); } - if (oq && fallback) { - let result: undefined | boolean | InternalRefetchQueriesResult; - // If onQueryUpdated is provided, we want to use it for all included - // queries, even the PureQueryOptions ones. Otherwise, we call the - // fallback function defined above. - if (onQueryUpdated) { - if (!diff) { - queryInfo.reset(); // Force queryInfo.getDiff() to read from cache. - diff = queryInfo.getDiff(); - } - result = onQueryUpdated(oq, diff, lastDiff); - } - if (!onQueryUpdated || result === true) { - result = fallback(); - } - if (result !== false) { - results.set(oq, result!); - } + if (result !== false) { + results.set(oq, result!); } }); } @@ -1452,30 +1478,3 @@ export class QueryManager { }; } } - -function getQueryIdsForQueryDescriptor( - qm: QueryManager, - desc: RefetchQueryDescriptor, -) { - const queryIds: string[] = []; - const isName = typeof desc === "string"; - if (isName || isDocumentNode(desc)) { - qm["queries"].forEach(({ observableQuery: oq, document }, queryId) => { - if (oq && - desc === (isName ? oq.queryName : document) && - oq.hasObservers()) { - queryIds.push(queryId); - } - }); - } else { - // We will be issuing a fresh network request for this query, so we - // pre-allocate a new query ID here. - queryIds.push(qm.generateQueryId()); - } - if (process.env.NODE_ENV !== "production" && !queryIds.length) { - invariant.warn(`Unknown query name ${ - JSON.stringify(desc) - } passed to refetchQueries method in options.include array`); - } - return queryIds; -} diff --git a/src/core/types.ts b/src/core/types.ts index a6907179a34..68fcf4d2f5f 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -23,7 +23,7 @@ export type OnQueryUpdated = ( ) => boolean | TResult; export type RefetchQueryDescriptor = string | DocumentNode | PureQueryOptions; -export type RefetchQueryDescription = RefetchQueryDescriptor[]; +export type RefetchQueryDescription = RefetchQueryDescriptor[] | "all" | "active"; // Used by ApolloClient["refetchQueries"] // TODO Improve documentation comments for this public type. From 5baa91807efcac312d139cba2659fabc15632757 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 2 Jun 2021 17:10:12 -0400 Subject: [PATCH 3/9] Reenable warning about included-but-unused query names/documents. --- src/core/QueryManager.ts | 33 +++++++++++++++++------- src/core/__tests__/QueryManager/index.ts | 6 ++--- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 9933501f43a..07667dd5f7e 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -737,16 +737,13 @@ export class QueryManager { include: RefetchQueryDescription = "active", ) { const queries = new Map>(); - const queryNames = new Set(); - const queryDocs = new Set(); + const queryNamesAndDocs = new Map(); const legacyQueryOptions = new Set(); if (Array.isArray(include)) { include.forEach(desc => { - if (typeof desc === "string") { - queryNames.add(desc); - } else if (isDocumentNode(desc)) { - queryDocs.add(desc); + if (typeof desc === "string" || isDocumentNode(desc)) { + queryNamesAndDocs.set(desc, false); } else if (isNonNullObject(desc) && desc.query) { legacyQueryOptions.add(desc); } @@ -760,7 +757,11 @@ export class QueryManager { return; } - const { fetchPolicy } = oq.options; + const { + queryName, + options: { fetchPolicy }, + } = oq; + if (fetchPolicy === "cache-only" || fetchPolicy === "standby" || !oq.hasObservers()) { @@ -770,10 +771,12 @@ export class QueryManager { if ( include === "active" || - queryNames.has(oq.queryName!) || - (document && queryDocs.has(document)) + (queryName && queryNamesAndDocs.has(queryName)) || + (document && queryNamesAndDocs.has(document)) ) { queries.set(queryId, oq); + if (queryName) queryNamesAndDocs.set(queryName, true); + if (document) queryNamesAndDocs.set(document, true); } } }); @@ -800,6 +803,18 @@ export class QueryManager { }); } + if (process.env.NODE_ENV !== "production" && queryNamesAndDocs.size) { + queryNamesAndDocs.forEach((included, nameOrDoc) => { + if (!included) { + invariant.warn(`Unknown query ${ + typeof nameOrDoc === "string" ? "named " : "" + }${ + JSON.stringify(nameOrDoc, null, 2) + } requested in refetchQueries options.include array`); + } + }); + } + return queries; } diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 56b7bc397c1..5af44167783 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -4774,8 +4774,7 @@ describe('QueryManager', () => { result => { expect(stripSymbols(result.data)).toEqual(secondReqData); expect(consoleWarnSpy).toHaveBeenLastCalledWith( - 'Unknown query name "fakeQuery" passed to refetchQueries method ' + - "in options.include array" + 'Unknown query named "fakeQuery" requested in refetchQueries options.include array' ); }, ).then(resolve, reject); @@ -4843,8 +4842,7 @@ describe('QueryManager', () => { }); }).then(() => { expect(consoleWarnSpy).toHaveBeenLastCalledWith( - 'Unknown query name "getAuthors" passed to refetchQueries method ' + - "in options.include array" + 'Unknown query named "getAuthors" requested in refetchQueries options.include array' ); }).then(resolve, reject); }); From 748c90c6f1c02b7946dbb3a09293d890d0fa9edb Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 2 Jun 2021 17:54:41 -0400 Subject: [PATCH 4/9] Clean up [Internal]RefetchQueriesInclude and related types. --- src/core/ApolloClient.ts | 19 ++++++++-------- src/core/QueryManager.ts | 20 ++++++++--------- src/core/types.ts | 41 +++++++++++++++++++++-------------- src/core/watchQueryOptions.ts | 6 ++--- src/react/types/types.ts | 4 ++-- 5 files changed, 49 insertions(+), 41 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index 58297858ca0..9c63094c9bd 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -18,7 +18,7 @@ import { RefetchQueriesOptions, RefetchQueriesResult, InternalRefetchQueriesResult, - RefetchQueryDescription, + RefetchQueriesInclude, } from './types'; import { @@ -566,17 +566,16 @@ export class ApolloClient implements DataProxy { } /** - * Get all currently active ObservableQuery objects, in a Map keyed by query - * ID strings. An "active" query is one that has observers and a fetchPolicy - * other than "standby" or "cache-only". You can include all ObservableQuery - * objects (including the inactive ones) by passing "all" instead of "active", - * or you can include just a subset of active queries by passing an array of - * query names or DocumentNode objects. This method is used internally by - * `client.refetchQueries` to handle its `include` option. + * Get all currently active `ObservableQuery` objects, in a `Map` keyed by + * query ID strings. An "active" query is one that has observers and a + * `fetchPolicy` other than "standby" or "cache-only". You can include all + * `ObservableQuery` objects (including the inactive ones) by passing "all" + * instead of "active", or you can include just a subset of active queries by + * passing an array of query names or DocumentNode objects. */ public getObservableQueries( - include: RefetchQueryDescription = "active", - ) { + include: RefetchQueriesInclude = "active", + ): Map> { return this.queryManager.getObservableQueries(include); } diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 07667dd5f7e..b6001129fbf 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -39,7 +39,7 @@ import { OperationVariables, MutationUpdaterFunction, OnQueryUpdated, - RefetchQueryDescription, + InternalRefetchQueriesInclude, InternalRefetchQueriesOptions, InternalRefetchQueriesResult, InternalRefetchQueriesMap, @@ -327,7 +327,7 @@ export class QueryManager { updateQueries: UpdateQueries; update?: MutationUpdaterFunction; awaitRefetchQueries?: boolean; - refetchQueries?: RefetchQueryDescription; + refetchQueries?: InternalRefetchQueriesInclude; removeOptimistic?: string; onQueryUpdated?: OnQueryUpdated; keepRootFields?: boolean; @@ -734,7 +734,7 @@ export class QueryManager { } public getObservableQueries( - include: RefetchQueryDescription = "active", + include: InternalRefetchQueriesInclude = "active", ) { const queries = new Map>(); const queryNamesAndDocs = new Map(); @@ -1180,7 +1180,7 @@ export class QueryManager { onQueryUpdated, }: InternalRefetchQueriesOptions, TResult> ): InternalRefetchQueriesMap { - const includedQueries = new Map; lastDiff?: Cache.DiffResult; diff?: Cache.DiffResult; @@ -1188,7 +1188,7 @@ export class QueryManager { if (include) { this.getObservableQueries(include).forEach((oq, queryId) => { - includedQueries.set(queryId, { + includedQueriesById.set(queryId, { oq, lastDiff: this.getQuery(queryId).getDiff(), }); @@ -1251,7 +1251,7 @@ export class QueryManager { // Since we're about to handle this query now, remove it from // includedQueriesById, in case it was added earlier because of // options.include. - includedQueries.delete(oq.queryId); + includedQueriesById.delete(oq.queryId); let result: boolean | InternalRefetchQueriesResult = onQueryUpdated(oq, diff, lastDiff); @@ -1277,19 +1277,19 @@ export class QueryManager { // If we don't have an onQueryUpdated function, and onQueryUpdated // was not disabled by passing null, make sure this query is // "included" like any other options.include-specified query. - includedQueries.set(oq.queryId, { oq, lastDiff, diff }); + includedQueriesById.set(oq.queryId, { oq, lastDiff, diff }); } } }, }); } - if (includedQueries.size) { - includedQueries.forEach(({ oq, lastDiff, diff }, queryId) => { + if (includedQueriesById.size) { + includedQueriesById.forEach(({ oq, lastDiff, diff }, queryId) => { let result: undefined | boolean | InternalRefetchQueriesResult; // If onQueryUpdated is provided, we want to use it for all included - // queries, even the PureQueryOptions ones. + // queries, even the QueryOptions ones. if (onQueryUpdated) { if (!diff) { const info = oq["queryInfo"]; diff --git a/src/core/types.ts b/src/core/types.ts index 68fcf4d2f5f..83e0963cb2b 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -7,6 +7,7 @@ import { QueryInfo } from './QueryInfo'; import { NetworkStatus } from './networkStatus'; import { Resolver } from './LocalState'; import { ObservableQuery } from './ObservableQuery'; +import { QueryOptions } from './watchQueryOptions'; import { Cache } from '../cache'; import { IsStrictlyAny } from '../utilities'; @@ -22,8 +23,18 @@ export type OnQueryUpdated = ( lastDiff: Cache.DiffResult | undefined, ) => boolean | TResult; -export type RefetchQueryDescriptor = string | DocumentNode | PureQueryOptions; -export type RefetchQueryDescription = RefetchQueryDescriptor[] | "all" | "active"; +export type RefetchQueryDescriptor = string | DocumentNode; +export type InternalRefetchQueryDescriptor = RefetchQueryDescriptor | QueryOptions; + +type RefetchQueriesIncludeShorthand = "all" | "active"; + +export type RefetchQueriesInclude = + | RefetchQueryDescriptor[] + | RefetchQueriesIncludeShorthand; + +export type InternalRefetchQueriesInclude = + | InternalRefetchQueryDescriptor[] + | RefetchQueriesIncludeShorthand; // Used by ApolloClient["refetchQueries"] // TODO Improve documentation comments for this public type. @@ -32,11 +43,11 @@ export interface RefetchQueriesOptions< TResult, > { updateCache?: (cache: TCache) => void; - // Although you can pass PureQueryOptions objects in addition to strings in - // the refetchQueries array for a mutation, the client.refetchQueries method - // deliberately discourages passing PureQueryOptions, by restricting the - // public type of the options.include array to string[] (just query names). - include?: Exclude[]; + // The client.refetchQueries method discourages passing QueryOptions, by + // restricting the public type of options.include to exclude QueryOptions as + // an available array element type (see InternalRefetchQueriesInclude for a + // version of RefetchQueriesInclude that allows legacy QueryOptions objects). + include?: RefetchQueriesInclude; optimistic?: boolean; // If no onQueryUpdated function is provided, any queries affected by the // updateCache function or included in the options.include array will be @@ -93,9 +104,10 @@ export interface InternalRefetchQueriesOptions< TCache extends ApolloCache, TResult, > extends Omit, "include"> { - // Just like the refetchQueries array for a mutation, allowing both strings - // and PureQueryOptions objects. - include?: RefetchQueryDescription; + // Just like the refetchQueries option for a mutation, an array of strings, + // DocumentNode objects, and/or QueryOptions objects, or one of the shorthand + // strings "all" or "active", to select every (active) query. + include?: InternalRefetchQueriesInclude; // This part of the API is a (useful) implementation detail, but need not be // exposed in the public client.refetchQueries API (above). removeOptimistic?: string; @@ -108,13 +120,10 @@ export type InternalRefetchQueriesMap = Map, InternalRefetchQueriesResult>; -export type OperationVariables = Record; +// TODO Remove this unnecessary type in Apollo Client 4. +export type { QueryOptions as PureQueryOptions }; -export type PureQueryOptions = { - query: DocumentNode; - variables?: { [key: string]: any }; - context?: any; -}; +export type OperationVariables = Record; export type ApolloQueryResult = { data: T; diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index 69e77888279..d66d8850503 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -8,7 +8,7 @@ import { OperationVariables, MutationUpdaterFunction, OnQueryUpdated, - RefetchQueryDescription, + InternalRefetchQueriesInclude, } from './types'; import { ApolloCache } from '../cache'; @@ -221,8 +221,8 @@ export interface MutationBaseOptions< * once these queries return. */ refetchQueries?: - | ((result: FetchResult) => RefetchQueryDescription) - | RefetchQueryDescription; + | ((result: FetchResult) => InternalRefetchQueriesInclude) + | InternalRefetchQueriesInclude; /** * By default, `refetchQueries` does not wait for the refetched queries to diff --git a/src/react/types/types.ts b/src/react/types/types.ts index 12e2d7aa12a..cd2bae7271f 100644 --- a/src/react/types/types.ts +++ b/src/react/types/types.ts @@ -17,7 +17,7 @@ import { NetworkStatus, ObservableQuery, OperationVariables, - PureQueryOptions, + InternalRefetchQueriesInclude, WatchQueryOptions, } from '../../core'; @@ -137,7 +137,7 @@ export type QueryTuple = [ export type RefetchQueriesFunction = ( ...args: any[] -) => Array; +) => InternalRefetchQueriesInclude; export interface BaseMutationOptions< TData, From 4e022bcf1feea1df6bbf8c15f981b5f2531eca8a Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 2 Jun 2021 18:16:07 -0400 Subject: [PATCH 5/9] Tests of the new "all" and "active" values for options.include. --- src/__tests__/refetchQueries.ts | 165 ++++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index d5b5bd72236..88725bbd997 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -351,6 +351,171 @@ describe("client.refetchQueries", () => { resolve(); }); + itAsync('includes all queries when options.include === "all"', async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const ayyResults = await client.refetchQueries({ + include: "all", + + updateCache(cache) { + cache.writeQuery({ + query: aQuery, + data: { + a: "Ayy", + }, + }); + }, + + onQueryUpdated(obs, diff) { + if (obs === aObs) { + expect(diff.result).toEqual({ a: "Ayy" }); + } else if (obs === bObs) { + expect(diff.result).toEqual({ b: "B" }); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "Ayy", b: "B" }); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + return Promise.resolve(diff.result); + }, + }); + + sortObjects(ayyResults); + + expect(ayyResults).toEqual([ + { a: "Ayy" }, + { a: "Ayy", b: "B" }, + { b: "B" }, + ]); + + const beeResults = await client.refetchQueries({ + include: "all", + + updateCache(cache) { + cache.writeQuery({ + query: bQuery, + data: { + b: "Bee", + }, + }); + }, + + onQueryUpdated(obs, diff) { + if (obs === aObs) { + expect(diff.result).toEqual({ a: "Ayy" }); + } else if (obs === bObs) { + expect(diff.result).toEqual({ b: "Bee" }); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "Ayy", b: "Bee" }); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + return diff.result; + }, + }); + + sortObjects(beeResults); + + expect(beeResults).toEqual([ + { a: "Ayy" }, + { a: "Ayy", b: "Bee" }, + { b: "Bee" }, + ]); + + unsubscribe(); + resolve(); + }); + + itAsync('includes all active queries when options.include === "active"', async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const extraObs = client.watchQuery({ query: abQuery }); + expect(extraObs.hasObservers()).toBe(false); + + const activeResults = await client.refetchQueries({ + include: "active", + + onQueryUpdated(obs, diff) { + if (obs === aObs) { + expect(diff.result).toEqual({ a: "A" }); + } else if (obs === bObs) { + expect(diff.result).toEqual({ b: "B" }); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "A", b: "B" }); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + return Promise.resolve(diff.result); + }, + }); + + sortObjects(activeResults); + + expect(activeResults).toEqual([ + { a: "A" }, + { a: "A", b: "B" }, + { b: "B" }, + ]); + + subs.push(extraObs.subscribe({ + next(result) { + expect(result).toEqual({ a: "A", b: "B" }); + }, + })); + expect(extraObs.hasObservers()).toBe(true); + + const resultsAfterSubscribe = await client.refetchQueries({ + include: "active", + + onQueryUpdated(obs, diff) { + if (obs === aObs) { + expect(diff.result).toEqual({ a: "A" }); + } else if (obs === bObs) { + expect(diff.result).toEqual({ b: "B" }); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "A", b: "B" }); + } else if (obs === extraObs) { + expect(diff.result).toEqual({ a: "A", b: "B" }); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + return Promise.resolve(diff.result); + }, + }); + + sortObjects(resultsAfterSubscribe); + + expect(resultsAfterSubscribe).toEqual([ + { a: "A" }, + { a: "A", b: "B" }, + // Included thanks to extraObs this time. + { a: "A", b: "B" }, + // Sorted last by sortObjects. + { b: "B" }, + ]); + + unsubscribe(); + resolve(); + }); + itAsync("refetches watched queries if onQueryUpdated not provided", async (resolve, reject) => { const client = makeClient(); const [ From ba92852c521517abd9e802e4d2900decf621541d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 2 Jun 2021 19:17:45 -0400 Subject: [PATCH 6/9] Test that both "all" and "active" ignore one-off client.query queries. --- src/__tests__/refetchQueries.ts | 107 +++++++++++++++++++++++++++++++- 1 file changed, 105 insertions(+), 2 deletions(-) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index 88725bbd997..a18f8f5a861 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -50,8 +50,13 @@ describe("client.refetchQueries", () => { operation.operationName.split("").forEach(letter => { data[letter.toLowerCase()] = letter.toUpperCase(); }); - observer.next({ data }); - observer.complete(); + function finish() { + observer.next({ data }); + observer.complete(); + } + if (typeof operation.variables.delay === "number") { + setTimeout(finish, operation.variables.delay); + } else finish(); })), }); } @@ -516,6 +521,104 @@ describe("client.refetchQueries", () => { resolve(); }); + itAsync('should not include unwatched single queries', async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const delayedQuery = gql`query DELAYED { d e l a y e d }`; + + client.query({ + query: delayedQuery, + variables: { + // Delay this query by 10 seconds so it stays in-flight. + delay: 10000, + }, + }).catch(reject); + + const queries = client["queryManager"]["queries"]; + expect(queries.size).toBe(4); + + queries.forEach((queryInfo, queryId) => { + if ( + queryId === "1" || + queryId === "2" || + queryId === "3" + ) { + expect(queryInfo.observableQuery).toBeInstanceOf(ObservableQuery); + } else if (queryId === "4") { + // One-off client.query-style queries never get an ObservableQuery, so + // they should not be included by include: "active". + expect(queryInfo.observableQuery).toBe(null); + expect(queryInfo.document).toBe(delayedQuery); + } + }); + + const activeResults = await client.refetchQueries({ + include: "active", + + onQueryUpdated(obs, diff) { + if (obs === aObs) { + expect(diff.result).toEqual({ a: "A" }); + } else if (obs === bObs) { + expect(diff.result).toEqual({ b: "B" }); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "A", b: "B" }); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + return Promise.resolve(diff.result); + }, + }); + + sortObjects(activeResults); + + expect(activeResults).toEqual([ + { a: "A" }, + { a: "A", b: "B" }, + { b: "B" }, + ]); + + const allResults = await client.refetchQueries({ + include: "all", + + onQueryUpdated(obs, diff) { + if (obs === aObs) { + expect(diff.result).toEqual({ a: "A" }); + } else if (obs === bObs) { + expect(diff.result).toEqual({ b: "B" }); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "A", b: "B" }); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + return Promise.resolve(diff.result); + }, + }); + + sortObjects(allResults); + + expect(allResults).toEqual([ + { a: "A" }, + { a: "A", b: "B" }, + { b: "B" }, + ]); + + unsubscribe(); + client.stop(); + + expect(queries.size).toBe(0); + + resolve(); + }); + itAsync("refetches watched queries if onQueryUpdated not provided", async (resolve, reject) => { const client = makeClient(); const [ From f6cf7f979265b3dc087def3988baeb6d04915cb2 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 4 Jun 2021 11:09:05 -0400 Subject: [PATCH 7/9] Make "active" exclude cache-only queries only for reFetchObservableQueries. Queries with `fetchPolicy: cache-only` are certainly not "inactive" (you just might not want to refetch them from the network), so excluding them is a choice for the reFetchObservableQueries method to make. Note: we are currently planning to deprecate reFetchObservableQueries and remove it in Apollo Client v4, so this logic will disappear from the codebase after that. The new client.refetchQueries API provides a much more flexible way to specify which queries you want to include, including the ability to filter/skip them in arbitrary ways in the onQueryUpdated function. --- src/core/QueryManager.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index b6001129fbf..177fdeb9774 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -762,9 +762,7 @@ export class QueryManager { options: { fetchPolicy }, } = oq; - if (fetchPolicy === "cache-only" || - fetchPolicy === "standby" || - !oq.hasObservers()) { + if (fetchPolicy === "standby" || !oq.hasObservers()) { // Skip inactive queries unless include === "all". return; } @@ -826,8 +824,13 @@ export class QueryManager { this.getObservableQueries( includeStandby ? "all" : "active" ).forEach((observableQuery, queryId) => { + const { fetchPolicy } = observableQuery.options; observableQuery.resetLastResults(); - observableQueryPromises.push(observableQuery.refetch()); + if (includeStandby || + (fetchPolicy !== "standby" && + fetchPolicy !== "cache-only")) { + observableQueryPromises.push(observableQuery.refetch()); + } this.getQuery(queryId).setDiff(null); }); From 1c6314c5a8c968efd072eca4d468c271873eb555 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 4 Jun 2021 11:43:54 -0400 Subject: [PATCH 8/9] Improve legacy refetchQueries test using subscribeAndCount. --- src/core/__tests__/QueryManager/index.ts | 39 +++++++++--------------- 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 5af44167783..30141e5d0c7 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -4904,31 +4904,22 @@ describe('QueryManager', () => { }, ); const observable = queryManager.watchQuery({ query, variables }); - let count = 0; - observable.subscribe({ - next: result => { - const resultData = stripSymbols(result.data); - if (count === 0) { - expect(resultData).toEqual(data); - queryManager.mutate({ - mutation, - variables: mutationVariables, - refetchQueries: [{ query, variables }], - }); - } - if (count === 1) { - setTimeout(() => { - expect(stripSymbols(observable.getCurrentResult().data)).toEqual( - secondReqData, - ); - resolve(); - }, 1); - - expect(resultData).toEqual(secondReqData); - } - count++; - }, + subscribeAndCount(reject, observable, (count, result) => { + if (count === 1) { + expect(result.data).toEqual(data); + queryManager.mutate({ + mutation, + variables: mutationVariables, + refetchQueries: [{ query, variables }], + }); + } else if (count === 2) { + expect(result.data).toEqual(secondReqData); + expect(observable.getCurrentResult().data).toEqual(secondReqData); + setTimeout(resolve, 10); + } else { + reject("too many results"); + } }); }); From 4a8e265016feaa4e5ac2e3820ae89a3ef145216e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 4 Jun 2021 11:49:41 -0400 Subject: [PATCH 9/9] Clean up legacy one-time queries after refetching in refetchQueries. Although passing one-time { query, variables } QueryOptions in the options.include array is discouraged by the type system, it's still allowed for the refetchQueries option for mutations, so we should make sure to stop those temporary queries after they've been fetched. I didn't want to complicate the client.getObservableQueries API to return additional metadata about which queries are legacy/temporary, so I'm using query ID strings (the keys of the getObservableQueries Map) to convey that information. --- src/core/QueryManager.ts | 9 +++++++-- src/core/__tests__/QueryManager/index.ts | 9 ++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 177fdeb9774..8e56d630f5d 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -782,8 +782,9 @@ export class QueryManager { if (legacyQueryOptions.size) { legacyQueryOptions.forEach((options: QueryOptions) => { // We will be issuing a fresh network request for this query, so we - // pre-allocate a new query ID here. - const queryId = this.generateQueryId(); + // pre-allocate a new query ID here, using a special prefix to enable + // cleaning up these temporary queries later, after fetching. + const queryId = makeUniqueId("legacyOneTimeQuery"); const queryInfo = this.getQuery(queryId).init({ document: options.query, variables: options.variables, @@ -1310,6 +1311,10 @@ export class QueryManager { if (result !== false) { results.set(oq, result!); } + + if (queryId.indexOf("legacyOneTimeQuery") >= 0) { + this.stopQueryNoBroadcast(queryId); + } }); } diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 30141e5d0c7..a474a3fd378 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -4916,7 +4916,14 @@ describe('QueryManager', () => { } else if (count === 2) { expect(result.data).toEqual(secondReqData); expect(observable.getCurrentResult().data).toEqual(secondReqData); - setTimeout(resolve, 10); + + return new Promise(res => setTimeout(res, 10)).then(() => { + // Make sure the QueryManager cleans up legacy one-time queries like + // the one we requested above using refetchQueries. + queryManager["queries"].forEach((queryInfo, queryId) => { + expect(queryId).not.toContain("legacyOneTimeQuery"); + }); + }).then(resolve, reject); } else { reject("too many results"); }