From eddad5a2ada64c32057f72a61ab4a151d9d85042 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Apr 2021 15:04:05 -0400 Subject: [PATCH 01/47] Rename cache.batch onDirty option to onWatchUpdated. Dirtiness is an overloaded metaphor that isn't widely used in our documentation or implementation (outside of the optimism npm package, which uses other terminology unrelated to Apollo Client). The cache.batch options.onWatchUpdated callback fires for any cache.watch watches whose queries were dirtied/affected/updated/invalidated/etc. by the options.transaction function. Choosing among these possible terms, I think "updated" is the most neutral and least likely to mislead. --- src/cache/core/cache.ts | 2 +- src/cache/inmemory/__tests__/cache.ts | 26 ++++++------- src/cache/inmemory/inMemoryCache.ts | 56 ++++++++++++++------------- src/core/QueryManager.ts | 2 +- src/core/__tests__/ObservableQuery.ts | 8 ++-- 5 files changed, 48 insertions(+), 46 deletions(-) diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts index 4c188fe83d9..84118a70006 100644 --- a/src/cache/core/cache.ts +++ b/src/cache/core/cache.ts @@ -26,7 +26,7 @@ export type BatchOptions> = { // If you want to find out which watched queries were invalidated during // this batch operation, pass this optional callback function. Returning // false from the callback will prevent broadcasting this result. - onDirty?: ( + onWatchUpdated?: ( this: C, watch: Cache.WatchOptions, diff: Cache.DiffResult, diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts index 08fc4766c44..6751b46fd8f 100644 --- a/src/cache/inmemory/__tests__/cache.ts +++ b/src/cache/inmemory/__tests__/cache.ts @@ -1348,7 +1348,7 @@ describe('Cache', () => { return { diffs, watch: options, cancel }; } - it('calls onDirty for each invalidated watch', () => { + it('calls onWatchUpdated for each invalidated watch', () => { const cache = new InMemoryCache; const aQuery = gql`query { a }`; @@ -1371,7 +1371,7 @@ describe('Cache', () => { }); }, optimistic: true, - onDirty(w, diff) { + onWatchUpdated(w, diff) { dirtied.set(w, diff); }, }); @@ -1412,7 +1412,7 @@ describe('Cache', () => { }); }, optimistic: true, - onDirty(w, diff) { + onWatchUpdated(w, diff) { dirtied.set(w, diff); }, }); @@ -1485,7 +1485,7 @@ describe('Cache', () => { }); }, optimistic: true, - onDirty(w, diff) { + onWatchUpdated(w, diff) { dirtied.set(w, diff); }, }); @@ -1506,7 +1506,7 @@ describe('Cache', () => { bInfo.cancel(); }); - it('does not pass previously invalidated queries to onDirty', () => { + it('does not pass previously invalidated queries to onWatchUpdated', () => { const cache = new InMemoryCache; const aQuery = gql`query { a }`; @@ -1527,13 +1527,13 @@ describe('Cache', () => { cache.writeQuery({ query: bQuery, - // Writing this data with broadcast:false queues this update for the - // next broadcast, whenever it happens. If that next broadcast is the - // one triggered by cache.batch, the bQuery broadcast could be - // accidentally intercepted by onDirty, even though the transaction - // does not touch the Query.b field. To solve this problem, the batch - // method calls cache.broadcastWatches() before the transaction, when - // options.onDirty is provided. + // Writing this data with broadcast:false queues this update for + // the next broadcast, whenever it happens. If that next broadcast + // is the one triggered by cache.batch, the bQuery broadcast could + // be accidentally intercepted by onWatchUpdated, even though the + // transaction does not touch the Query.b field. To solve this + // problem, the batch method calls cache.broadcastWatches() before + // the transaction, when options.onWatchUpdated is provided. broadcast: false, data: { b: "beeeee", @@ -1559,7 +1559,7 @@ describe('Cache', () => { }); }, optimistic: true, - onDirty(watch, diff) { + onWatchUpdated(watch, diff) { dirtied.set(watch, diff); }, }); diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 0f059478c0d..668850c9ac7 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -39,8 +39,8 @@ export interface InMemoryCacheConfig extends ApolloReducerConfig { type BroadcastOptions = Pick< BatchOptions, - | "onDirty" | "optimistic" + | "onWatchUpdated" > const defaultConfig: InMemoryCacheConfig = { @@ -359,22 +359,23 @@ export class InMemoryCache extends ApolloCache { } }; - const { onDirty } = options; + const { onWatchUpdated } = options; const alreadyDirty = new Set(); - if (onDirty && !this.txCount) { - // If an options.onDirty callback is provided, we want to call it with - // only the Cache.WatchOptions objects affected by options.transaction, - // but there might be dirty watchers already waiting to be broadcast that - // have nothing to do with the transaction. To prevent including those - // watchers in the post-transaction broadcast, we perform this initial - // broadcast to collect the dirty watchers, so we can re-dirty them later, - // after the post-transaction broadcast, allowing them to receive their - // pending broadcasts the next time broadcastWatches is called, just as - // they would if we never called cache.batch. + if (onWatchUpdated && !this.txCount) { + // If an options.onWatchUpdated callback is provided, we want to + // call it with only the Cache.WatchOptions objects affected by + // options.transaction, but there might be dirty watchers already + // waiting to be broadcast that have nothing to do with the + // transaction. To prevent including those watchers in the + // post-transaction broadcast, we perform this initial broadcast to + // collect the dirty watchers, so we can re-dirty them later, after + // the post-transaction broadcast, allowing them to receive their + // pending broadcasts the next time broadcastWatches is called, just + // as they would if we never called cache.batch. this.broadcastWatches({ ...options, - onDirty(watch) { + onWatchUpdated(watch) { alreadyDirty.add(watch); return false; }, @@ -402,18 +403,18 @@ export class InMemoryCache extends ApolloCache { // Note: if this.txCount > 0, then alreadyDirty.size === 0, so this code // takes the else branch and calls this.broadcastWatches(options), which // does nothing when this.txCount > 0. - if (onDirty && alreadyDirty.size) { + if (onWatchUpdated && alreadyDirty.size) { this.broadcastWatches({ ...options, - onDirty(watch, diff) { - const onDirtyResult = onDirty.call(this, watch, diff); - if (onDirtyResult !== false) { - // Since onDirty did not return false, this diff is about to be - // broadcast to watch.callback, so we don't need to re-dirty it - // with the other alreadyDirty watches below. + onWatchUpdated(watch, diff) { + const result = onWatchUpdated.call(this, watch, diff); + if (result !== false) { + // Since onWatchUpdated did not return false, this diff is + // about to be broadcast to watch.callback, so we don't need + // to re-dirty it with the other alreadyDirty watches below. alreadyDirty.delete(watch); } - return onDirtyResult; + return result; } }); // Silently re-dirty any watches that were already dirty before the @@ -422,8 +423,9 @@ export class InMemoryCache extends ApolloCache { alreadyDirty.forEach(watch => this.maybeBroadcastWatch.dirty(watch)); } } else { - // If alreadyDirty is empty or we don't have an options.onDirty function, - // we don't need to go to the trouble of wrapping options.onDirty. + // If alreadyDirty is empty or we don't have an onWatchUpdated + // function, we don't need to go to the trouble of wrapping + // options.onWatchUpdated. this.broadcastWatches(options); } } @@ -482,10 +484,10 @@ export class InMemoryCache extends ApolloCache { diff.fromOptimisticTransaction = true; } - if (options.onDirty && - options.onDirty.call(this, c, diff) === false) { - // Returning false from the onDirty callback will prevent calling - // c.callback(diff) for this watcher. + if (options.onWatchUpdated && + options.onWatchUpdated.call(this, c, diff) === false) { + // Returning false from the onWatchUpdated callback will prevent + // calling c.callback(diff) for this watcher. return; } } diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index aabc1afd508..f8ab8c47cd4 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -382,7 +382,7 @@ export class QueryManager { // Write the final mutation.result to the root layer of the cache. optimistic: false, - onDirty: mutation.reobserveQuery && ((watch, diff) => { + onWatchUpdated: mutation.reobserveQuery && ((watch, diff) => { if (watch.watcher instanceof QueryInfo) { const oq = watch.watcher.observableQuery; if (oq) { diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index 423f13cf63d..5dc2903cc49 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -1953,7 +1953,7 @@ describe('ObservableQuery', () => { }); let invalidateCount = 0; - let onDirtyCount = 0; + let onWatchUpdatedCount = 0; cache.batch({ optimistic: true, @@ -1969,7 +1969,7 @@ describe('ObservableQuery', () => { }); }, // Verify that the cache.modify operation did trigger a cache broadcast. - onDirty(watch, diff) { + onWatchUpdated(watch, diff) { expect(watch.watcher).toBe(queryInfo); expect(diff).toEqual({ complete: true, @@ -1979,7 +1979,7 @@ describe('ObservableQuery', () => { }, }, }); - ++onDirtyCount; + ++onWatchUpdatedCount; }, }); @@ -1987,7 +1987,7 @@ describe('ObservableQuery', () => { expect(setDiffSpy).toHaveBeenCalledTimes(1); expect(notifySpy).not.toHaveBeenCalled(); expect(invalidateCount).toBe(1); - expect(onDirtyCount).toBe(1); + expect(onWatchUpdatedCount).toBe(1); queryManager.stop(); }).then(resolve, reject); } else { From 97857baf28843a0086e0c47482c9158e8d5ab8b2 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Apr 2021 15:18:33 -0400 Subject: [PATCH 02/47] Rename mutation reobserveQuery callback to onQueryUpdated. Previously, options.onDirty (for cache.batch) and options.reobserveQuery (for mutations) had no obvious relationship, even though one is implemented in terms of the other. Now, cache.batch takes an onWatchUpdated callback and mutations take onQueryUpdated, which I think better represents the similarity (as well as the crucial difference in argument types) between the callbacks. Also, the options.onQueryUpdated callback is typically triggered by the mutation's options.update function, so the "onQueryUpdated" terminology more accurately reflects the source of the updates. --- src/core/QueryManager.ts | 12 +++++------ src/core/__tests__/QueryManager/index.ts | 8 ++++---- src/core/types.ts | 2 +- src/core/watchQueryOptions.ts | 4 ++-- .../hooks/__tests__/useMutation.test.tsx | 20 +++++++++---------- src/react/types/types.ts | 6 +++--- 6 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index f8ab8c47cd4..1c37017c725 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -36,7 +36,7 @@ import { ApolloQueryResult, OperationVariables, MutationUpdaterFunction, - ReobserveQueryCallback, + OnQueryUpdated, } from './types'; import { LocalState } from './LocalState'; @@ -139,7 +139,7 @@ export class QueryManager { refetchQueries = [], awaitRefetchQueries = false, update: updateWithProxyFn, - reobserveQuery, + onQueryUpdated, errorPolicy = 'none', fetchPolicy, context, @@ -236,7 +236,7 @@ export class QueryManager { context, updateQueries, update: updateWithProxyFn, - reobserveQuery, + onQueryUpdated, }); } catch (e) { // Likewise, throwing an error from the asyncMap mapping function @@ -311,7 +311,7 @@ export class QueryManager { context?: TContext; updateQueries: UpdateQueries; update?: MutationUpdaterFunction; - reobserveQuery?: ReobserveQueryCallback; + onQueryUpdated?: OnQueryUpdated; }, cache = this.cache, ): Promise { @@ -382,11 +382,11 @@ export class QueryManager { // Write the final mutation.result to the root layer of the cache. optimistic: false, - onWatchUpdated: mutation.reobserveQuery && ((watch, diff) => { + onWatchUpdated: mutation.onQueryUpdated && ((watch, diff) => { if (watch.watcher instanceof QueryInfo) { const oq = watch.watcher.observableQuery; if (oq) { - reobserveResults.push(mutation.reobserveQuery!(oq, diff)); + reobserveResults.push(mutation.onQueryUpdated!(oq, diff)); // Prevent the normal cache broadcast of this result. return false; } diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index d869425b033..da43103d649 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -5227,7 +5227,7 @@ describe('QueryManager', () => { }); }); - describe('reobserveQuery', () => { + describe('onQueryUpdated', () => { const mutation = gql` mutation changeAuthorName { changeAuthorName(newName: "Jack Smith") { @@ -5316,7 +5316,7 @@ describe('QueryManager', () => { }); }, - reobserveQuery(obsQuery) { + onQueryUpdated(obsQuery) { expect(obsQuery.options.query).toBe(query); return obsQuery.refetch().then(async () => { // Wait a bit to make sure the mutation really awaited the @@ -5374,7 +5374,7 @@ describe('QueryManager', () => { }); }, - reobserveQuery(obsQuery) { + onQueryUpdated(obsQuery) { expect(obsQuery.options.query).toBe(query); return obsQuery.refetch(); }, @@ -5415,7 +5415,7 @@ describe('QueryManager', () => { cache.evict({ fieldName: "author" }); }, - reobserveQuery(obsQuery) { + onQueryUpdated(obsQuery) { expect(obsQuery.options.query).toBe(query); return obsQuery.reobserve({ fetchPolicy: "network-only", diff --git a/src/core/types.ts b/src/core/types.ts index c6e40a3eb31..fd08388656f 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -15,7 +15,7 @@ export type DefaultContext = Record; export type QueryListener = (queryInfo: QueryInfo) => void; -export type ReobserveQueryCallback = ( +export type OnQueryUpdated = ( observableQuery: ObservableQuery, diff: Cache.DiffResult, ) => void | Promise; diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index beca5cce0cc..f9f96975d80 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -8,7 +8,7 @@ import { PureQueryOptions, OperationVariables, MutationUpdaterFunction, - ReobserveQueryCallback, + OnQueryUpdated, } from './types'; import { ApolloCache } from '../cache'; @@ -260,7 +260,7 @@ export interface MutationBaseOptions< * A function that will be called for each ObservableQuery affected by * this mutation, after the mutation has completed. */ - reobserveQuery?: ReobserveQueryCallback; + onQueryUpdated?: OnQueryUpdated; /** * Specifies the {@link ErrorPolicy} to be used for this operation diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index f3f938e50c6..198f1fcdfa6 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -747,7 +747,7 @@ describe('useMutation Hook', () => { }); describe('refetching queries', () => { - itAsync('can pass reobserveQuery to useMutation', (resolve, reject) => { + itAsync('can pass onQueryUpdated to useMutation', (resolve, reject) => { interface TData { todoCount: number; } @@ -791,17 +791,17 @@ describe('useMutation Hook', () => { }).setOnError(reject), }); - // The goal of this test is to make sure reobserveQuery gets called as + // The goal of this test is to make sure onQueryUpdated gets called as // part of the createTodo mutation, so we use this reobservePromise to - // await the calling of reobserveQuery. - interface ReobserveResults { + // await the calling of onQueryUpdated. + interface OnQueryUpdatedResults { obsQuery: ObservableQuery; diff: Cache.DiffResult; result: ApolloQueryResult; } - let reobserveResolve: (results: ReobserveResults) => any; - const reobservePromise = new Promise(resolve => { - reobserveResolve = resolve; + let resolveOnUpdate: (results: OnQueryUpdatedResults) => any; + const onUpdatePromise = new Promise(resolve => { + resolveOnUpdate = resolve; }); let finishedReobserving = false; @@ -838,10 +838,10 @@ describe('useMutation Hook', () => { act(() => { createTodo({ variables, - reobserveQuery(obsQuery, diff) { + onQueryUpdated(obsQuery, diff) { return obsQuery.reobserve().then(result => { finishedReobserving = true; - reobserveResolve({ obsQuery, diff, result }); + resolveOnUpdate({ obsQuery, diff, result }); }); }, }); @@ -888,7 +888,7 @@ describe('useMutation Hook', () => { ); - return reobservePromise.then(results => { + return onUpdatePromise.then(results => { expect(finishedReobserving).toBe(true); expect(results.diff).toEqual({ diff --git a/src/react/types/types.ts b/src/react/types/types.ts index a7c692ba6f2..b3bef363669 100644 --- a/src/react/types/types.ts +++ b/src/react/types/types.ts @@ -19,7 +19,7 @@ import { ObservableQuery, OperationVariables, PureQueryOptions, - ReobserveQueryCallback, + OnQueryUpdated, WatchQueryFetchPolicy, WatchQueryOptions, } from '../../core'; @@ -154,7 +154,7 @@ export interface BaseMutationOptions< awaitRefetchQueries?: boolean; errorPolicy?: ErrorPolicy; update?: MutationUpdaterFunction; - reobserveQuery?: ReobserveQueryCallback; + onQueryUpdated?: OnQueryUpdated; client?: ApolloClient; notifyOnNetworkStatusChange?: boolean; context?: TContext; @@ -175,7 +175,7 @@ export interface MutationFunctionOptions< refetchQueries?: Array | RefetchQueriesFunction; awaitRefetchQueries?: boolean; update?: MutationUpdaterFunction; - reobserveQuery?: ReobserveQueryCallback; + onQueryUpdated?: OnQueryUpdated; context?: TContext; fetchPolicy?: WatchQueryFetchPolicy; } From 204ae7773468ad94ecdcc4109927706812c80ead Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Apr 2021 16:40:45 -0400 Subject: [PATCH 03/47] Remove optimistic layer at same time as final mutation update. --- src/cache/core/cache.ts | 2 ++ src/cache/inmemory/inMemoryCache.ts | 5 +++++ src/core/QueryManager.ts | 7 +++++++ 3 files changed, 14 insertions(+) diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts index 84118a70006..ac84eaf01b6 100644 --- a/src/cache/core/cache.ts +++ b/src/cache/core/cache.ts @@ -23,6 +23,8 @@ export type BatchOptions> = { // the same as passing null. optimistic: string | boolean; + removeOptimistic?: string; + // If you want to find out which watched queries were invalidated during // this batch operation, pass this optional callback function. Returning // false from the callback will prevent broadcasting this result. diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 668850c9ac7..77b6b1a29dd 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -342,6 +342,7 @@ export class InMemoryCache extends ApolloCache { const { transaction, optimistic = true, + removeOptimistic, } = options; const perform = (layer?: EntityStore) => { @@ -400,6 +401,10 @@ export class InMemoryCache extends ApolloCache { perform(); } + if (typeof removeOptimistic === "string") { + this.optimisticData = this.optimisticData.removeLayer(removeOptimistic); + } + // Note: if this.txCount > 0, then alreadyDirty.size === 0, so this code // takes the else branch and calls this.broadcastWatches(options), which // does nothing when this.txCount > 0. diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 1c37017c725..48d122e8d1c 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -232,6 +232,7 @@ export class QueryManager { result, document: mutation, variables, + removeOptimistic: !!optimisticResponse, errorPolicy, context, updateQueries, @@ -311,6 +312,7 @@ export class QueryManager { context?: TContext; updateQueries: UpdateQueries; update?: MutationUpdaterFunction; + removeOptimistic: boolean; onQueryUpdated?: OnQueryUpdated; }, cache = this.cache, @@ -382,6 +384,10 @@ export class QueryManager { // Write the final mutation.result to the root layer of the cache. optimistic: false, + removeOptimistic: mutation.removeOptimistic + ? mutation.mutationId + : void 0, + onWatchUpdated: mutation.onQueryUpdated && ((watch, diff) => { if (watch.watcher instanceof QueryInfo) { const oq = watch.watcher.observableQuery; @@ -420,6 +426,7 @@ export class QueryManager { try { this.markMutationResult({ ...mutation, + removeOptimistic: false, result: { data }, }, cache); } catch (error) { From 82c68c838978c87e95bbab8ef9e3194a9d844a77 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 6 Apr 2021 16:27:59 -0400 Subject: [PATCH 04/47] Improve options API of client.refetchQueries method. This is obviously a breaking change for client.refetchQueries, but that method was introduced in PR #7431, targeting the release-3.4 branch. Since Apollo Client v3.4 is still in beta, we still have room to rework the signature of the client.refetchQueries method (introduced in #7431 and released in @apollo/client@3.4.0-beta.3), not only adding significant new functionality like options.updateCache and options.onQueryUpdated, but also leaving room to add functionality more easily in the future, without breaking backwards compatibility, since client.refetchQueries takes named options and returns an object of named results, so adding new options or returning new results never needs to be a breaking change. --- src/__tests__/client.ts | 4 +- src/core/ApolloClient.ts | 29 ++- src/core/QueryManager.ts | 316 +++++++++++++++-------- src/core/__tests__/QueryManager/index.ts | 4 +- src/core/watchQueryOptions.ts | 8 + 5 files changed, 250 insertions(+), 111 deletions(-) diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index af13183cef1..6f0f58ecb61 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -2436,7 +2436,9 @@ describe('client', () => { // @ts-ignore const spy = jest.spyOn(client.queryManager, 'refetchQueries'); - await client.refetchQueries(['Author1']); + await client.refetchQueries({ + include: ['Author1'], + }); expect(spy).toHaveBeenCalled(); }); diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index a87b4e30395..26bc4013a6e 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -23,7 +23,7 @@ import { MutationOptions, SubscriptionOptions, WatchQueryFetchPolicy, - RefetchQueryDescription, + RefetchQueriesOptions, } from './watchQueryOptions'; import { @@ -536,9 +536,30 @@ export class ApolloClient implements DataProxy { * Takes optional parameter `includeStandby` which will include queries in standby-mode when refetching. */ public refetchQueries( - queries: RefetchQueryDescription, - ): Promise[]> { - return Promise.all(this.queryManager.refetchQueries(queries)); + options: Pick< + RefetchQueriesOptions>, + | "updateCache" + | "include" + | "optimistic" + | "onQueryUpdated" + >, + ): Promise<{ + queries: ObservableQuery[]; + results: Map, ApolloQueryResult>; + }> { + const results = this.queryManager.refetchQueries(options); + const queries: ObservableQuery[] = []; + const values: any[] = []; + + results.forEach((value, obsQuery) => { + queries.push(obsQuery); + values.push(value); + }); + + return Promise.all(values).then(values => { + values.forEach((value, i) => results.set(queries[i], value)); + return { queries, results }; + }); } /** diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 48d122e8d1c..59da8d3ec66 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -29,6 +29,7 @@ import { WatchQueryFetchPolicy, ErrorPolicy, RefetchQueryDescription, + RefetchQueriesOptions, } from './watchQueryOptions'; import { ObservableQuery } from './ObservableQuery'; import { NetworkStatus, isNetworkRequestInFlight } from './networkStatus'; @@ -194,8 +195,6 @@ export class QueryManager { const self = this; return new Promise((resolve, reject) => { - let storeResult: FetchResult | null; - return asyncMap( self.getObservableFromLink( mutation, @@ -219,37 +218,75 @@ export class QueryManager { mutationStoreValue.error = null; } - storeResult = result; - - if (fetchPolicy !== 'no-cache') { - try { - // Returning the result of markMutationResult here makes the - // mutation await any Promise that markMutationResult returns, - // since we are returning this Promise from the asyncMap mapping - // function. - return self.markMutationResult({ - mutationId, - result, - document: mutation, - variables, - removeOptimistic: !!optimisticResponse, - errorPolicy, - context, - updateQueries, - update: updateWithProxyFn, - onQueryUpdated, - }); - } catch (e) { - // Likewise, throwing an error from the asyncMap mapping function - // will result in calling the subscribed error handler function. - throw new ApolloError({ - networkError: e, - }); - } + const storeResult: typeof result = { ...result }; + + if (typeof refetchQueries === "function") { + refetchQueries = refetchQueries(storeResult); + } + + if (errorPolicy === 'ignore' && + graphQLResultHasError(storeResult)) { + delete storeResult.errors; + } + + if (fetchPolicy === 'no-cache') { + const refetchResults = this.refetchQueries({ + include: refetchQueries, + onQueryUpdated, + }); + + return Promise.all( + awaitRefetchQueries ? refetchResults.values() : [], + ).then(() => storeResult); } + + const markPromise = self.markMutationResult< + TData, + TVariables, + TContext, + TCache + >({ + mutationId, + result, + document: mutation, + variables, + errorPolicy, + context, + update: updateWithProxyFn, + updateQueries, + refetchQueries, + removeOptimistic: optimisticResponse ? mutationId : void 0, + onQueryUpdated, + }); + + if (awaitRefetchQueries || onQueryUpdated) { + // Returning the result of markMutationResult here makes the + // mutation await the Promise that markMutationResult returns, + // since we are returning markPromise from the map function + // we passed to asyncMap above. + return markPromise.then(() => storeResult); + } + + return storeResult; }, ).subscribe({ + next(storeResult) { + if (optimisticResponse) { + self.cache.removeOptimistic(mutationId); + } + + self.broadcastQueries(); + + // At the moment, a mutation can have only one result, so we can + // immediately resolve upon receiving the first result. In the future, + // mutations containing @defer or @stream directives might receive + // multiple FetchResult payloads from the ApolloLink chain, so we will + // probably need to collect those results in this next method and call + // resolve only later, in an observer.complete function. + resolve(storeResult); + }, + error(err: Error) { if (mutationStoreValue) { mutationStoreValue.loading = false; @@ -268,41 +305,16 @@ export class QueryManager { }), ); }, - - complete() { - if (optimisticResponse) { - self.cache.removeOptimistic(mutationId); - } - - self.broadcastQueries(); - - // allow for conditional refetches - // XXX do we want to make this the only API one day? - if (typeof refetchQueries === 'function') { - refetchQueries = refetchQueries(storeResult!); - } - - const refetchQueryPromises = self.refetchQueries(refetchQueries); - - Promise.all( - awaitRefetchQueries ? refetchQueryPromises : [], - ).then(() => { - if ( - errorPolicy === 'ignore' && - storeResult && - graphQLResultHasError(storeResult) - ) { - delete storeResult.errors; - } - - resolve(storeResult!); - }, reject); - }, }); }); } - public markMutationResult>( + public markMutationResult< + TData, + TVariables, + TContext, + TCache extends ApolloCache + >( mutation: { mutationId: string; result: FetchResult; @@ -312,7 +324,8 @@ export class QueryManager { context?: TContext; updateQueries: UpdateQueries; update?: MutationUpdaterFunction; - removeOptimistic: boolean; + refetchQueries?: RefetchQueryDescription; + removeOptimistic?: string; onQueryUpdated?: OnQueryUpdated; }, cache = this.cache, @@ -364,43 +377,37 @@ export class QueryManager { }); } - const reobserveResults: any[] = []; + const results = this.refetchQueries({ + updateCache(cache: TCache) { + cacheWrites.forEach(write => cache.write(write)); - cache.batch({ - transaction(c) { - cacheWrites.forEach(write => c.write(write)); // If the mutation has some writes associated with it then we need to // apply those writes to the store by running this reducer again with // a write action. const { update } = mutation; if (update) { - update(c as any, mutation.result, { + update(cache, mutation.result, { context: mutation.context, variables: mutation.variables, }); } }, + include: mutation.refetchQueries, + // Write the final mutation.result to the root layer of the cache. optimistic: false, - removeOptimistic: mutation.removeOptimistic - ? mutation.mutationId - : void 0, + // Remove the corresponding optimistic layer at the same time as we + // write the final non-optimistic result. + removeOptimistic: mutation.removeOptimistic, - onWatchUpdated: mutation.onQueryUpdated && ((watch, diff) => { - if (watch.watcher instanceof QueryInfo) { - const oq = watch.watcher.observableQuery; - if (oq) { - reobserveResults.push(mutation.onQueryUpdated!(oq, diff)); - // Prevent the normal cache broadcast of this result. - return false; - } - } - }), + // Let the caller of client.mutate optionally determine the refetching + // behavior for watched queries after the mutation.update function runs. + onQueryUpdated: mutation.onQueryUpdated, }); - return Promise.all(reobserveResults).then(() => void 0); + return Promise.all(results.values()).then(() => void 0); } return Promise.resolve(); @@ -426,7 +433,6 @@ export class QueryManager { try { this.markMutationResult({ ...mutation, - removeOptimistic: false, result: { data }, }, cache); } catch (error) { @@ -1023,38 +1029,138 @@ export class QueryManager { return concast; } - public refetchQueries( - queries: RefetchQueryDescription, - ): Promise>[] { - const refetchQueryPromises: Promise>[] = []; - - if (isNonEmptyArray(queries)) { - queries.forEach(refetchQuery => { - if (typeof refetchQuery === 'string') { - this.queries.forEach(({ observableQuery }) => { - if (observableQuery && - observableQuery.hasObservers() && - observableQuery.queryName === refetchQuery) { - refetchQueryPromises.push(observableQuery.refetch()); + public refetchQueries({ + updateCache, + include, + optimistic = false, + removeOptimistic = optimistic ? "TODO" : void 0, + onQueryUpdated, + }: RefetchQueriesOptions>) { + const includedQueriesById = new Map(); + const results = new Map, any>(); + + if (include) { + const queryIdsByQueryName: Record = Object.create(null); + this.queries.forEach((queryInfo, queryId) => { + const oq = queryInfo.observableQuery; + const queryName = oq && oq.queryName; + if (queryName) { + queryIdsByQueryName[queryName] = queryId; + } + }); + + include.forEach(queryNameOrOptions => { + if (typeof queryNameOrOptions === "string") { + const queryId = queryIdsByQueryName[queryNameOrOptions]; + if (queryId) { + includedQueriesById.set(queryId, queryNameOrOptions); + } else { + invariant.warn(`Unknown query name ${ + JSON.stringify(queryNameOrOptions) + } passed to refetchQueries method in options.include array`); + } + } else { + includedQueriesById.set( + // We will be issuing a fresh network request for this query, so we + // pre-allocate a new query ID here. + this.generateQueryId(), + queryNameOrOptions, + ); + } + }); + } + + if (updateCache) { + this.cache.batch({ + transaction: updateCache, + + // Since you can perform any combination of cache reads and/or writes in + // the cache.batch transaction function, its optimistic option can be + // either a boolean or a string, representing three distinct modes of + // operation: + // + // * false: read/write only the root layer + // * true: read/write the topmost layer + // * string: read/write a fresh optimistic layer with that ID string + // + // When typeof optimistic === "string", a new optimistic layer will be + // temporarily created within cache.batch with that string as its ID. If + // we then pass that same string as the removeOptimistic option, we can + // make cache.batch immediately remove the optimistic layer after + // running the transaction, triggering only one broadcast. + // + // However, the refetchQueries method accepts only true or false for its + // optimistic option (not string). We interpret true to mean a temporary + // optimistic layer should be created, to allow efficiently rolling back + // the effect of the updateCache function, which involves passing a + // string instead of true as the optimistic option to cache.batch, when + // refetchQueries receives optimistic: true. + // + // In other words, we are deliberately not supporting the use case of + // writing to an *existing* optimistic layer (using the refetchQueries + // updateCache function), since that would potentially interfere with + // other optimistic updates in progress. Instead, you can read/write + // only the root layer by passing optimistic: false to refetchQueries, + // or you can read/write a brand new optimistic layer that will be + // automatically removed by passing optimistic: true. + optimistic: optimistic && removeOptimistic || false, + + // The removeOptimistic option can also be provided by itself, even if + // optimistic === false, to remove some previously-added optimistic + // layer safely and efficiently, like we do in markMutationResult. + // + // If an explicit removeOptimistic string is provided with optimistic: + // true, the removeOptimistic string will determine the ID of the + // temporary optimistic layer, in case that ever matters. + removeOptimistic, + + onWatchUpdated: onQueryUpdated && function (watch, diff) { + if (watch.watcher instanceof QueryInfo) { + const oq = watch.watcher.observableQuery; + if (oq) { + includedQueriesById.delete(oq.queryId); + results.set(oq, onQueryUpdated!(oq, diff)); + // Prevent the normal cache broadcast of this result. + return false; } + } + }, + }); + } + + if (includedQueriesById.size) { + includedQueriesById.forEach((queryNameOrOptions, queryId) => { + const queryInfo = this.getQuery(queryId); + let oq = queryInfo.observableQuery; + if (oq) { + const result = onQueryUpdated + ? onQueryUpdated(oq, queryInfo.getDiff()) + : oq.refetch(); + + results.set(oq, result); + + } else if (typeof queryNameOrOptions === "object") { + const fetchPromise = this.fetchQuery(queryId, { + query: queryNameOrOptions.query, + variables: queryNameOrOptions.variables, + fetchPolicy: "network-only", + context: queryNameOrOptions.context, }); - } else { - const queryOptions: QueryOptions = { - query: refetchQuery.query, - variables: refetchQuery.variables, - fetchPolicy: 'network-only', - }; - - if (refetchQuery.context) { - queryOptions.context = refetchQuery.context; + + oq = queryInfo.observableQuery; + if (oq) { + results.set(oq, fetchPromise); + } else { + throw new InvariantError(JSON.stringify(queryInfo, null, 2)); } - refetchQueryPromises.push(this.query(queryOptions)); + const stop = () => this.stopQuery(queryId); + fetchPromise.then(stop, stop); } }); } - return refetchQueryPromises; + return results; } private fetchQueryByPolicy( diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index da43103d649..e14effcf004 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -4382,7 +4382,9 @@ describe('QueryManager', () => { observable.subscribe({ next: () => null }); observable2.subscribe({ next: () => null }); - return Promise.all(queryManager.refetchQueries(['GetAuthor', 'GetAuthor2'])).then(() => { + return Promise.all(queryManager.refetchQueries({ + include: ['GetAuthor', 'GetAuthor2'], + })).then(() => { const result = getCurrentQueryResult(observable); expect(result.partial).toBe(false); expect(stripSymbols(result.data)).toEqual(dataChanged); diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index f9f96975d80..c2fd7b74cc0 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -191,6 +191,14 @@ export interface SubscriptionOptions; +export type RefetchQueriesOptions> = { + updateCache?: (cache: Cache) => void; + include?: RefetchQueryDescription; + optimistic?: boolean; + removeOptimistic?: string; + onQueryUpdated?: OnQueryUpdated; +}; + export interface MutationBaseOptions< TData = any, TVariables = OperationVariables, From 55bc6bede21c57b1f9265f1287ac762fcfd8c7b8 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 8 Apr 2021 16:25:49 -0400 Subject: [PATCH 05/47] Ensure optimistic layers get removed in QueryManager#refetchQueries. --- src/core/QueryManager.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 59da8d3ec66..f0392f050bd 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -272,10 +272,6 @@ export class QueryManager { ).subscribe({ next(storeResult) { - if (optimisticResponse) { - self.cache.removeOptimistic(mutationId); - } - self.broadcastQueries(); // At the moment, a mutation can have only one result, so we can @@ -1160,6 +1156,17 @@ export class QueryManager { }); } + if (removeOptimistic) { + // In case no updateCache callback was provided (so cache.batch was not + // called above, and thus did not already remove the optimistic layer), + // remove it here. Since this is a no-op when the layer has already been + // removed, we do it even if we called cache.batch above, since it's + // possible this.cache is an instance of some ApolloCache subclass other + // than InMemoryCache, and does not fully support the removeOptimistic + // option for cache.batch. + this.cache.removeOptimistic(removeOptimistic); + } + return results; } From b8c18d48d26c64ffc238e9dc2dc78a56f1db91c3 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 8 Apr 2021 18:36:02 -0400 Subject: [PATCH 06/47] Fix bug due to Promise.all not understanding iterators. --- src/core/QueryManager.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index f0392f050bd..9f9dd5d4978 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -230,13 +230,15 @@ export class QueryManager { } if (fetchPolicy === 'no-cache') { - const refetchResults = this.refetchQueries({ + const results: any[] = []; + + this.refetchQueries({ include: refetchQueries, onQueryUpdated, - }); + }).forEach(result => results.push(result)); return Promise.all( - awaitRefetchQueries ? refetchResults.values() : [], + awaitRefetchQueries ? results : [], ).then(() => storeResult); } @@ -373,7 +375,9 @@ export class QueryManager { }); } - const results = this.refetchQueries({ + const results: any[] = []; + + this.refetchQueries({ updateCache(cache: TCache) { cacheWrites.forEach(write => cache.write(write)); @@ -401,9 +405,10 @@ export class QueryManager { // Let the caller of client.mutate optionally determine the refetching // behavior for watched queries after the mutation.update function runs. onQueryUpdated: mutation.onQueryUpdated, - }); - return Promise.all(results.values()).then(() => void 0); + }).forEach(result => results.push(result)); + + return Promise.all(results).then(() => void 0); } return Promise.resolve(); From 3936f04842eb9b57fcda4cead477ecb336e19181 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 8 Apr 2021 18:46:26 -0400 Subject: [PATCH 07/47] Fix test failing due to incorrect usage of queryManager.refetchQueries. --- src/core/__tests__/QueryManager/index.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index e14effcf004..3748c78a98a 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -4382,9 +4382,12 @@ describe('QueryManager', () => { observable.subscribe({ next: () => null }); observable2.subscribe({ next: () => null }); - return Promise.all(queryManager.refetchQueries({ + const results: any[] = []; + queryManager.refetchQueries({ include: ['GetAuthor', 'GetAuthor2'], - })).then(() => { + }).forEach(result => results.push(result)); + + return Promise.all(results).then(() => { const result = getCurrentQueryResult(observable); expect(result.partial).toBe(false); expect(stripSymbols(result.data)).toEqual(dataChanged); From d0962618b7029653c6d04beaab32bbaee0404cef Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 8 Apr 2021 19:08:56 -0400 Subject: [PATCH 08/47] Test that refetchQueries now warns when named query not found. --- src/core/__tests__/QueryManager/index.ts | 47 +++++++++++------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 3748c78a98a..fdf0294bdc3 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -4629,16 +4629,12 @@ describe('QueryManager', () => { }); describe('refetchQueries', () => { - const oldWarn = console.warn; - let timesWarned = 0; - + let consoleWarnSpy: jest.SpyInstance; beforeEach(() => { - // clear warnings - timesWarned = 0; - // mock warn method - console.warn = (...args: any[]) => { - timesWarned++; - }; + consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(); + }); + afterEach(() => { + consoleWarnSpy.mockRestore(); }); itAsync('should refetch the right query when a result is successfully returned', (resolve, reject) => { @@ -4777,12 +4773,15 @@ describe('QueryManager', () => { }, result => { expect(stripSymbols(result.data)).toEqual(secondReqData); - expect(timesWarned).toBe(0); + expect(consoleWarnSpy).toHaveBeenLastCalledWith( + 'Unknown query name "fakeQuery" passed to refetchQueries method ' + + "in options.include array" + ); }, ).then(resolve, reject); }); - itAsync('should ignore without warning a query name that is asked to refetch with no active subscriptions', (resolve, reject) => { + itAsync('should ignore (with warning) a query named in refetchQueries that has no active subscriptions', (resolve, reject) => { const mutation = gql` mutation changeAuthorName { changeAuthorName(newName: "Jack Smith") { @@ -4836,16 +4835,18 @@ describe('QueryManager', () => { const observable = queryManager.watchQuery({ query }); return observableToPromise({ observable }, result => { expect(stripSymbols(result.data)).toEqual(data); - }) - .then(() => { - // The subscription has been stopped already - return queryManager.mutate({ - mutation, - refetchQueries: ['getAuthors'], - }); - }) - .then(() => expect(timesWarned).toBe(0)) - .then(resolve, reject); + }).then(() => { + // The subscription has been stopped already + return queryManager.mutate({ + mutation, + refetchQueries: ['getAuthors'], + }); + }).then(() => { + expect(consoleWarnSpy).toHaveBeenLastCalledWith( + 'Unknown query name "getAuthors" passed to refetchQueries method ' + + "in options.include array" + ); + }).then(resolve, reject); }); itAsync('also works with a query document and variables', (resolve, reject) => { @@ -5226,10 +5227,6 @@ describe('QueryManager', () => { }, ).then(resolve, reject); }); - - afterEach(() => { - console.warn = oldWarn; - }); }); describe('onQueryUpdated', () => { From c760500ed6bcf5eb3d42df4d5fe412cf74abe633 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 8 Apr 2021 19:52:27 -0400 Subject: [PATCH 09/47] Ensure ObservableQuery is created for PureQueryOptions refetches. --- src/core/QueryManager.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 9f9dd5d4978..e240df11a95 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1141,20 +1141,21 @@ export class QueryManager { results.set(oq, result); } else if (typeof queryNameOrOptions === "object") { - const fetchPromise = this.fetchQuery(queryId, { + const options: WatchQueryOptions = { query: queryNameOrOptions.query, variables: queryNameOrOptions.variables, fetchPolicy: "network-only", context: queryNameOrOptions.context, - }); + }; - oq = queryInfo.observableQuery; - if (oq) { - results.set(oq, fetchPromise); - } else { - throw new InvariantError(JSON.stringify(queryInfo, null, 2)); - } + queryInfo.setObservableQuery(oq = new ObservableQuery({ + queryManager: this, + queryInfo, + options, + })); + const fetchPromise = this.fetchQuery(queryId, options); + results.set(oq, fetchPromise); const stop = () => this.stopQuery(queryId); fetchPromise.then(stop, stop); } From 21f09869945ef9a03800c514ef14591a5c83361f Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 9 Apr 2021 16:41:53 -0400 Subject: [PATCH 10/47] Separate {Public,Private}RefetchQueriesOptions. --- src/core/ApolloClient.ts | 30 ++++++++----------- src/core/QueryManager.ts | 15 ++++++---- src/core/__tests__/QueryManager/index.ts | 3 +- src/core/types.ts | 6 ++-- src/core/watchQueryOptions.ts | 30 +++++++++++++++---- .../hooks/__tests__/useMutation.test.tsx | 1 + src/react/types/types.ts | 12 ++++++-- 7 files changed, 62 insertions(+), 35 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index 26bc4013a6e..8abcc95fd18 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -23,7 +23,7 @@ import { MutationOptions, SubscriptionOptions, WatchQueryFetchPolicy, - RefetchQueriesOptions, + PublicRefetchQueriesOptions, } from './watchQueryOptions'; import { @@ -535,31 +535,25 @@ export class ApolloClient implements DataProxy { * active queries. * Takes optional parameter `includeStandby` which will include queries in standby-mode when refetching. */ - public refetchQueries( - options: Pick< - RefetchQueriesOptions>, - | "updateCache" - | "include" - | "optimistic" - | "onQueryUpdated" - >, + public refetchQueries( + options: PublicRefetchQueriesOptions>, ): Promise<{ queries: ObservableQuery[]; - results: Map, ApolloQueryResult>; + results: ApolloQueryResult[]; }> { - const results = this.queryManager.refetchQueries(options); + const map = this.queryManager.refetchQueries(options); const queries: ObservableQuery[] = []; - const values: any[] = []; + const results: any[] = []; - results.forEach((value, obsQuery) => { + map.forEach((result, obsQuery) => { queries.push(obsQuery); - values.push(value); + results.push(result); }); - return Promise.all(values).then(values => { - values.forEach((value, i) => results.set(queries[i], value)); - return { queries, results }; - }); + return Promise.all(results).then(results => ({ + queries, + results, + })); } /** diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index e240df11a95..081a0023787 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -29,7 +29,7 @@ import { WatchQueryFetchPolicy, ErrorPolicy, RefetchQueryDescription, - RefetchQueriesOptions, + PrivateRefetchQueriesOptions, } from './watchQueryOptions'; import { ObservableQuery } from './ObservableQuery'; import { NetworkStatus, isNetworkRequestInFlight } from './networkStatus'; @@ -132,7 +132,12 @@ export class QueryManager { this.fetchCancelFns.clear(); } - public async mutate>({ + public async mutate< + TData, + TVariables, + TContext, + TCache extends ApolloCache + >({ mutation, variables, optimisticResponse, @@ -324,7 +329,7 @@ export class QueryManager { update?: MutationUpdaterFunction; refetchQueries?: RefetchQueryDescription; removeOptimistic?: string; - onQueryUpdated?: OnQueryUpdated; + onQueryUpdated?: OnQueryUpdated; }, cache = this.cache, ): Promise { @@ -1030,13 +1035,13 @@ export class QueryManager { return concast; } - public refetchQueries({ + public refetchQueries({ updateCache, include, optimistic = false, removeOptimistic = optimistic ? "TODO" : void 0, onQueryUpdated, - }: RefetchQueriesOptions>) { + }: PrivateRefetchQueriesOptions>) { const includedQueriesById = new Map(); const results = new Map, any>(); diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index fdf0294bdc3..56b7bc397c1 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -5320,11 +5320,12 @@ describe('QueryManager', () => { onQueryUpdated(obsQuery) { expect(obsQuery.options.query).toBe(query); - return obsQuery.refetch().then(async () => { + return obsQuery.refetch().then(async (result) => { // Wait a bit to make sure the mutation really awaited the // refetching of the query. await new Promise(resolve => setTimeout(resolve, 100)); finishedRefetch = true; + return result; }); }, }).then(() => { diff --git a/src/core/types.ts b/src/core/types.ts index fd08388656f..566d1c232c5 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -15,10 +15,10 @@ export type DefaultContext = Record; export type QueryListener = (queryInfo: QueryInfo) => void; -export type OnQueryUpdated = ( +export type OnQueryUpdated = ( observableQuery: ObservableQuery, - diff: Cache.DiffResult, -) => void | Promise; + diff: Cache.DiffResult, +) => void | Promise>; export type OperationVariables = Record; diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index c2fd7b74cc0..ac8c1cccfcc 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -191,13 +191,31 @@ export interface SubscriptionOptions; -export type RefetchQueriesOptions> = { - updateCache?: (cache: Cache) => void; - include?: RefetchQueryDescription; +export interface PublicRefetchQueriesOptions< + TData, + TCache extends ApolloCache, +> { + 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?: string[]; optimistic?: boolean; + onQueryUpdated?: OnQueryUpdated; +} + +export interface PrivateRefetchQueriesOptions< + TData, + TCache extends ApolloCache, +> extends Omit, "include"> { + // Just like the refetchQueries array for a mutation, allowing both strings + // and PureQueryOptions objects. + include?: RefetchQueryDescription; + // This part of the API is a (useful) implementation detail, but need not be + // exposed in the public client.refetchQueries API (above). removeOptimistic?: string; - onQueryUpdated?: OnQueryUpdated; -}; +} export interface MutationBaseOptions< TData = any, @@ -268,7 +286,7 @@ export interface MutationBaseOptions< * A function that will be called for each ObservableQuery affected by * this mutation, after the mutation has completed. */ - onQueryUpdated?: OnQueryUpdated; + onQueryUpdated?: OnQueryUpdated; /** * Specifies the {@link ErrorPolicy} to be used for this operation diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index 198f1fcdfa6..01f51941862 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -842,6 +842,7 @@ describe('useMutation Hook', () => { return obsQuery.reobserve().then(result => { finishedReobserving = true; resolveOnUpdate({ obsQuery, diff, result }); + return result; }); }, }); diff --git a/src/react/types/types.ts b/src/react/types/types.ts index b3bef363669..9d165d1e97f 100644 --- a/src/react/types/types.ts +++ b/src/react/types/types.ts @@ -154,7 +154,11 @@ export interface BaseMutationOptions< awaitRefetchQueries?: boolean; errorPolicy?: ErrorPolicy; update?: MutationUpdaterFunction; - onQueryUpdated?: OnQueryUpdated; + // Use OnQueryUpdated instead of OnQueryUpdated here because TData + // is the shape of the mutation result, but onQueryUpdated gets called with + // results from any queries affected by the mutation update function, which + // probably do not have the same shape as the mutation result. + onQueryUpdated?: OnQueryUpdated; client?: ApolloClient; notifyOnNetworkStatusChange?: boolean; context?: TContext; @@ -175,7 +179,11 @@ export interface MutationFunctionOptions< refetchQueries?: Array | RefetchQueriesFunction; awaitRefetchQueries?: boolean; update?: MutationUpdaterFunction; - onQueryUpdated?: OnQueryUpdated; + // Use OnQueryUpdated instead of OnQueryUpdated here because TData + // is the shape of the mutation result, but onQueryUpdated gets called with + // results from any queries affected by the mutation update function, which + // probably do not have the same shape as the mutation result. + onQueryUpdated?: OnQueryUpdated; context?: TContext; fetchPolicy?: WatchQueryFetchPolicy; } From b94956e4b6018dabe1138fd6b964a00ff8c71f7e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 16 Apr 2021 16:12:39 -0400 Subject: [PATCH 11/47] Simplify client.refetchQueries return type. Some consumers may not care what client.refetchQueries returns (so we shouldn't do a lot of work to return something they won't use), while others may want to await Promise.all(client.refetchQueries(...).updates), while others may want direct access to client.refetchQueries(...).updates and/or .queries, without waiting on a promise. The { queries, updates } result contains adequate information for all of these use cases, and leaves room to introduce more properties in the future, if we need to support additional client.refetchQueries use cases. --- src/core/ApolloClient.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index 8abcc95fd18..ed1fe05e365 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -535,25 +535,25 @@ export class ApolloClient implements DataProxy { * active queries. * Takes optional parameter `includeStandby` which will include queries in standby-mode when refetching. */ - public refetchQueries( - options: PublicRefetchQueriesOptions>, - ): Promise<{ + public refetchQueries< + TData, + TCache extends ApolloCache = ApolloCache, + >( + options: PublicRefetchQueriesOptions, + ): { queries: ObservableQuery[]; - results: ApolloQueryResult[]; - }> { + updates: any[]; + } { const map = this.queryManager.refetchQueries(options); const queries: ObservableQuery[] = []; - const results: any[] = []; + const updates: any[] = []; map.forEach((result, obsQuery) => { queries.push(obsQuery); - results.push(result); + updates.push(result); }); - return Promise.all(results).then(results => ({ - queries, - results, - })); + return { queries, updates }; } /** From 87dddf224a3ad22b36c2429151533aac86a1d83a Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 16 Apr 2021 16:13:16 -0400 Subject: [PATCH 12/47] Allow onQueryUpdated callbacks to return a boolean. --- src/cache/core/cache.ts | 2 +- src/core/QueryManager.ts | 45 +++++++++++++++++++++++++++------------- src/core/types.ts | 2 +- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts index ac84eaf01b6..2517c368306 100644 --- a/src/cache/core/cache.ts +++ b/src/cache/core/cache.ts @@ -32,7 +32,7 @@ export type BatchOptions> = { this: C, watch: Cache.WatchOptions, diff: Cache.DiffResult, - ) => void | false; + ) => any; }; export abstract class ApolloCache implements DataProxy { diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 081a0023787..2cc854eb385 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1045,6 +1045,23 @@ export class QueryManager { const includedQueriesById = new Map(); const results = new Map, any>(); + function maybeAddResult(oq: ObservableQuery, result: any): boolean { + // The onQueryUpdated function can return false to ignore this query and + // skip its normal broadcast, or true to allow the usual broadcast to + // happen (when diff.result has changed). + if (result === false || result === true) { + return result; + } + + // Returning anything other than true or false from onQueryUpdated will + // cause the result to be included in the results Map, while also + // canceling/overriding the normal broadcast. + results.set(oq, result); + + // Prevent the normal cache broadcast of this result. + return false; + } + if (include) { const queryIdsByQueryName: Record = Object.create(null); this.queries.forEach((queryInfo, queryId) => { @@ -1121,14 +1138,13 @@ export class QueryManager { removeOptimistic, onWatchUpdated: onQueryUpdated && function (watch, diff) { - if (watch.watcher instanceof QueryInfo) { - const oq = watch.watcher.observableQuery; - if (oq) { - includedQueriesById.delete(oq.queryId); - results.set(oq, onQueryUpdated!(oq, diff)); - // Prevent the normal cache broadcast of this result. - return false; - } + const oq = + watch.watcher instanceof QueryInfo && + watch.watcher.observableQuery; + + if (oq) { + includedQueriesById.delete(oq.queryId); + return maybeAddResult(oq, onQueryUpdated(oq, diff)); } }, }); @@ -1139,11 +1155,12 @@ export class QueryManager { const queryInfo = this.getQuery(queryId); let oq = queryInfo.observableQuery; if (oq) { - const result = onQueryUpdated - ? onQueryUpdated(oq, queryInfo.getDiff()) - : oq.refetch(); - - results.set(oq, result); + maybeAddResult( + oq, + onQueryUpdated + ? onQueryUpdated(oq, queryInfo.getDiff()) + : oq.refetch(), + ); } else if (typeof queryNameOrOptions === "object") { const options: WatchQueryOptions = { @@ -1160,7 +1177,7 @@ export class QueryManager { })); const fetchPromise = this.fetchQuery(queryId, options); - results.set(oq, fetchPromise); + maybeAddResult(oq, fetchPromise); const stop = () => this.stopQuery(queryId); fetchPromise.then(stop, stop); } diff --git a/src/core/types.ts b/src/core/types.ts index 566d1c232c5..1528e466c9d 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -18,7 +18,7 @@ export type QueryListener = (queryInfo: QueryInfo) => void; export type OnQueryUpdated = ( observableQuery: ObservableQuery, diff: Cache.DiffResult, -) => void | Promise>; +) => boolean | Promise>; export type OperationVariables = Record; From 8ef7d650c29b50319582bc56a2c5df64c828e640 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 16 Apr 2021 16:42:43 -0400 Subject: [PATCH 13/47] Pick unique optimistic layer ID for optimistic client.refetchQueries. TODO This functionality (the options.optimistic option for client.refetchQueries) needs more test coverage. --- src/core/QueryManager.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 2cc854eb385..69ac833c751 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1039,7 +1039,7 @@ export class QueryManager { updateCache, include, optimistic = false, - removeOptimistic = optimistic ? "TODO" : void 0, + removeOptimistic = optimistic ? makeUniqueId("refetchQueries") : void 0, onQueryUpdated, }: PrivateRefetchQueriesOptions>) { const includedQueriesById = new Map(); @@ -1367,3 +1367,10 @@ export class QueryManager { }; } } + +const prefixCounts: Record = Object.create(null); +function makeUniqueId(prefix: string) { + const count = prefixCounts[prefix] || 1; + prefixCounts[prefix] = count + 1; + return `${prefix}:${count}:${Math.random().toString(36).slice(2)}`; +} From 1282b91c96271fdd5c1101496221b5112fc3d216 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 19 Apr 2021 11:33:19 -0400 Subject: [PATCH 14/47] Pass previous diff to watch.callback, onWatchUpdated, and onQueryUpdated. --- src/cache/core/cache.ts | 3 ++- src/cache/core/types/Cache.ts | 5 ++++- src/cache/inmemory/inMemoryCache.ts | 7 ++++--- src/core/QueryManager.ts | 4 ++-- src/core/types.ts | 3 ++- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts index 2517c368306..0dac825ef13 100644 --- a/src/cache/core/cache.ts +++ b/src/cache/core/cache.ts @@ -31,7 +31,8 @@ export type BatchOptions> = { onWatchUpdated?: ( this: C, watch: Cache.WatchOptions, - diff: Cache.DiffResult, + newDiff: Cache.DiffResult, + oldDiff?: Cache.DiffResult, ) => any; }; diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts index 98b0fb00f48..35bd4e1aab5 100644 --- a/src/cache/core/types/Cache.ts +++ b/src/cache/core/types/Cache.ts @@ -2,7 +2,10 @@ import { DataProxy } from './DataProxy'; import { Modifier, Modifiers } from './common'; export namespace Cache { - export type WatchCallback = (diff: Cache.DiffResult) => void; + export type WatchCallback = ( + newDiff: Cache.DiffResult, + oldDiff?: Cache.DiffResult, + ) => void; export interface ReadOptions extends DataProxy.Query { diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 77b6b1a29dd..d289b7ac213 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -477,6 +477,7 @@ export class InMemoryCache extends ApolloCache { c: Cache.WatchOptions, options?: BroadcastOptions, ) { + const { lastDiff } = c; const diff = this.diff({ query: c.query, variables: c.variables, @@ -490,15 +491,15 @@ export class InMemoryCache extends ApolloCache { } if (options.onWatchUpdated && - options.onWatchUpdated.call(this, c, diff) === false) { + options.onWatchUpdated.call(this, c, diff, lastDiff) === false) { // Returning false from the onWatchUpdated callback will prevent // calling c.callback(diff) for this watcher. return; } } - if (!c.lastDiff || c.lastDiff.result !== diff.result) { - c.callback(c.lastDiff = diff); + if (!lastDiff || lastDiff.result !== diff.result) { + c.callback(c.lastDiff = diff, lastDiff); } } } diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 69ac833c751..92ddf6e21d1 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1137,14 +1137,14 @@ export class QueryManager { // temporary optimistic layer, in case that ever matters. removeOptimistic, - onWatchUpdated: onQueryUpdated && function (watch, diff) { + onWatchUpdated: onQueryUpdated && function (watch, newDiff, oldDiff) { const oq = watch.watcher instanceof QueryInfo && watch.watcher.observableQuery; if (oq) { includedQueriesById.delete(oq.queryId); - return maybeAddResult(oq, onQueryUpdated(oq, diff)); + return maybeAddResult(oq, onQueryUpdated(oq, newDiff, oldDiff)); } }, }); diff --git a/src/core/types.ts b/src/core/types.ts index 1528e466c9d..14333ba02a8 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -17,7 +17,8 @@ export type QueryListener = (queryInfo: QueryInfo) => void; export type OnQueryUpdated = ( observableQuery: ObservableQuery, - diff: Cache.DiffResult, + newDiff: Cache.DiffResult, + oldDiff?: Cache.DiffResult, ) => boolean | Promise>; export type OperationVariables = Record; From 4b66ff286669240b5257bc2c3638e1ac241cb1dc Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 19 Apr 2021 11:45:17 -0400 Subject: [PATCH 15/47] Use deep equality check before calling watch.callback. Similar to #7997, but further upstream. --- src/cache/inmemory/inMemoryCache.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index d289b7ac213..e1c97759441 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -3,6 +3,7 @@ import './fixPolyfills'; import { DocumentNode } from 'graphql'; import { OptimisticWrapperFunction, wrap } from 'optimism'; +import { equal } from '@wry/equality'; import { ApolloCache, BatchOptions } from '../core/cache'; import { Cache } from '../core/types/Cache'; @@ -498,7 +499,7 @@ export class InMemoryCache extends ApolloCache { } } - if (!lastDiff || lastDiff.result !== diff.result) { + if (!lastDiff || !equal(lastDiff.result, diff.result)) { c.callback(c.lastDiff = diff, lastDiff); } } From 58d320b70ac7fdf03289738a0ef6fee7216b7472 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 20 Apr 2021 10:01:45 -0400 Subject: [PATCH 16/47] Improve spy mocking for queryManager.refetchQueries test. https://github.com/apollographql/apollo-client/pull/8000#discussion_r616266128 --- src/__tests__/client.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 6f0f58ecb61..446693a53d9 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -2428,18 +2428,18 @@ describe('client', () => { }); it('has a refetchQueries method which calls QueryManager', async () => { - // TODO(dannycochran) const client = new ApolloClient({ link: ApolloLink.empty(), cache: new InMemoryCache(), }); - // @ts-ignore - const spy = jest.spyOn(client.queryManager, 'refetchQueries'); - await client.refetchQueries({ - include: ['Author1'], - }); - expect(spy).toHaveBeenCalled(); + const spy = jest.spyOn(client['queryManager'], 'refetchQueries'); + spy.mockImplementation(() => new Map); + + const options = { include: ['Author1'] }; + await client.refetchQueries(options); + + expect(spy).toHaveBeenCalledWith(options); }); itAsync('should propagate errors from network interface to observers', (resolve, reject) => { From 897a4ff559a4a9fc4e75f816cb742259fb6d6015 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 20 Apr 2021 10:20:44 -0400 Subject: [PATCH 17/47] Promote BatchOptions from ApolloCache module to Cache.BatchOptions. https://github.com/apollographql/apollo-client/pull/8000#discussion_r616275019 --- src/cache/core/cache.ts | 27 +---------------------- src/cache/core/types/Cache.ts | 34 +++++++++++++++++++++++++++++ src/cache/inmemory/inMemoryCache.ts | 6 ++--- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts index 0dac825ef13..441d49a1ef0 100644 --- a/src/cache/core/cache.ts +++ b/src/cache/core/cache.ts @@ -11,31 +11,6 @@ import { Cache } from './types/Cache'; export type Transaction = (c: ApolloCache) => void; -export type BatchOptions> = { - // Same as the first parameter of performTransaction, except the cache - // argument will have the subclass type rather than ApolloCache. - transaction(cache: C): void; - - // Passing a string for this option creates a new optimistic layer with - // that string as its layer.id, just like passing a string for the - // optimisticId parameter of performTransaction. Passing true is the - // same as passing undefined to performTransaction, and passing false is - // the same as passing null. - optimistic: string | boolean; - - removeOptimistic?: string; - - // If you want to find out which watched queries were invalidated during - // this batch operation, pass this optional callback function. Returning - // false from the callback will prevent broadcasting this result. - onWatchUpdated?: ( - this: C, - watch: Cache.WatchOptions, - newDiff: Cache.DiffResult, - oldDiff?: Cache.DiffResult, - ) => any; -}; - export abstract class ApolloCache implements DataProxy { // required to implement // core API @@ -84,7 +59,7 @@ export abstract class ApolloCache implements DataProxy { // provide a default batch implementation that's just another way of calling // performTransaction. Subclasses of ApolloCache (such as InMemoryCache) can // override the batch method to do more interesting things with its options. - public batch(options: BatchOptions) { + public batch(options: Cache.BatchOptions) { const optimisticId = typeof options.optimistic === "string" ? options.optimistic : options.optimistic === false ? null : void 0; diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts index 35bd4e1aab5..cba9499ec0a 100644 --- a/src/cache/core/types/Cache.ts +++ b/src/cache/core/types/Cache.ts @@ -1,5 +1,6 @@ import { DataProxy } from './DataProxy'; import { Modifier, Modifiers } from './common'; +import { ApolloCache } from '../cache'; export namespace Cache { export type WatchCallback = ( @@ -53,6 +54,39 @@ export namespace Cache { broadcast?: boolean; } + export interface BatchOptions> { + // Same as the first parameter of performTransaction, except the cache + // argument will have the subclass type rather than ApolloCache. + transaction(cache: C): void; + + // Passing a string for this option creates a new optimistic layer, with the + // given string as its layer.id, just like passing a string for the + // optimisticId parameter of performTransaction. Passing true is the same as + // passing undefined to performTransaction (runing the batch operation + // against the current top layer of the cache), and passing false is the + // same as passing null (running the operation against root/non-optimistic + // cache data). + optimistic: string | boolean; + + // If you specify the ID of an optimistic layer using this option, that + // layer will be removed as part of the batch transaction, triggering at + // most one broadcast for both the transaction and the removal of the layer. + // Note: this option is needed because calling cache.removeOptimistic during + // the transaction function may not be not safe, since any modifications to + // cache layers may be discarded after the transaction finishes. + removeOptimistic?: string; + + // If you want to find out which watched queries were invalidated during + // this batch operation, pass this optional callback function. Returning + // false from the callback will prevent broadcasting this result. + onWatchUpdated?: ( + this: C, + watch: Cache.WatchOptions, + diff: Cache.DiffResult, + lastDiff: Cache.DiffResult | undefined, + ) => any; + } + export import DiffResult = DataProxy.DiffResult; export import ReadQueryOptions = DataProxy.ReadQueryOptions; export import ReadFragmentOptions = DataProxy.ReadFragmentOptions; diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index e1c97759441..d5f6e557a03 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -5,7 +5,7 @@ import { DocumentNode } from 'graphql'; import { OptimisticWrapperFunction, wrap } from 'optimism'; import { equal } from '@wry/equality'; -import { ApolloCache, BatchOptions } from '../core/cache'; +import { ApolloCache } from '../core/cache'; import { Cache } from '../core/types/Cache'; import { MissingFieldError } from '../core/types/common'; import { @@ -39,7 +39,7 @@ export interface InMemoryCacheConfig extends ApolloReducerConfig { } type BroadcastOptions = Pick< - BatchOptions, + Cache.BatchOptions, | "optimistic" | "onWatchUpdated" > @@ -339,7 +339,7 @@ export class InMemoryCache extends ApolloCache { private txCount = 0; - public batch(options: BatchOptions) { + public batch(options: Cache.BatchOptions) { const { transaction, optimistic = true, From 6eb4d2c9539e670b6e1623e7d31f0119c05c5f2e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 20 Apr 2021 10:34:14 -0400 Subject: [PATCH 18/47] Reuse diff/lastDiff terminology, instead of newDiff/oldDiff. https://github.com/apollographql/apollo-client/pull/8000#discussion_r616276917 --- src/cache/core/types/Cache.ts | 4 ++-- src/core/QueryManager.ts | 4 ++-- src/core/types.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts index cba9499ec0a..b9a1b713cb0 100644 --- a/src/cache/core/types/Cache.ts +++ b/src/cache/core/types/Cache.ts @@ -4,8 +4,8 @@ import { ApolloCache } from '../cache'; export namespace Cache { export type WatchCallback = ( - newDiff: Cache.DiffResult, - oldDiff?: Cache.DiffResult, + diff: Cache.DiffResult, + lastDiff?: Cache.DiffResult, ) => void; export interface ReadOptions diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 92ddf6e21d1..8c7afe75714 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1137,14 +1137,14 @@ export class QueryManager { // temporary optimistic layer, in case that ever matters. removeOptimistic, - onWatchUpdated: onQueryUpdated && function (watch, newDiff, oldDiff) { + onWatchUpdated: onQueryUpdated && function (watch, diff, lastDiff) { const oq = watch.watcher instanceof QueryInfo && watch.watcher.observableQuery; if (oq) { includedQueriesById.delete(oq.queryId); - return maybeAddResult(oq, onQueryUpdated(oq, newDiff, oldDiff)); + return maybeAddResult(oq, onQueryUpdated(oq, diff, lastDiff)); } }, }); diff --git a/src/core/types.ts b/src/core/types.ts index 14333ba02a8..e849fa85875 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -17,8 +17,8 @@ export type QueryListener = (queryInfo: QueryInfo) => void; export type OnQueryUpdated = ( observableQuery: ObservableQuery, - newDiff: Cache.DiffResult, - oldDiff?: Cache.DiffResult, + diff: Cache.DiffResult, + lastDiff?: Cache.DiffResult, ) => boolean | Promise>; export type OperationVariables = Record; From 30fe80969958da6eb4e09173cbaae442d8b53dd9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 20 Apr 2021 10:39:23 -0400 Subject: [PATCH 19/47] Consolidate options destructuring. https://github.com/apollographql/apollo-client/pull/8000#discussion_r616270942 --- src/cache/inmemory/inMemoryCache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index d5f6e557a03..660c9bb22de 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -344,6 +344,7 @@ export class InMemoryCache extends ApolloCache { transaction, optimistic = true, removeOptimistic, + onWatchUpdated, } = options; const perform = (layer?: EntityStore) => { @@ -361,7 +362,6 @@ export class InMemoryCache extends ApolloCache { } }; - const { onWatchUpdated } = options; const alreadyDirty = new Set(); if (onWatchUpdated && !this.txCount) { From f913260913881bc00d599cbda5008468afcd4dfb Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 20 Apr 2021 11:31:19 -0400 Subject: [PATCH 20/47] {Public,Private}RefetchQueriesOptions => {,Internal}RefetchQueriesOptions https://github.com/apollographql/apollo-client/pull/8000#discussion_r616766562 --- src/core/ApolloClient.ts | 4 ++-- src/core/QueryManager.ts | 4 ++-- src/core/watchQueryOptions.ts | 9 ++++++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index ed1fe05e365..7bf6185b84f 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -23,7 +23,7 @@ import { MutationOptions, SubscriptionOptions, WatchQueryFetchPolicy, - PublicRefetchQueriesOptions, + RefetchQueriesOptions, } from './watchQueryOptions'; import { @@ -539,7 +539,7 @@ export class ApolloClient implements DataProxy { TData, TCache extends ApolloCache = ApolloCache, >( - options: PublicRefetchQueriesOptions, + options: RefetchQueriesOptions, ): { queries: ObservableQuery[]; updates: any[]; diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 8c7afe75714..a10e044878f 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -29,7 +29,7 @@ import { WatchQueryFetchPolicy, ErrorPolicy, RefetchQueryDescription, - PrivateRefetchQueriesOptions, + InternalRefetchQueriesOptions, } from './watchQueryOptions'; import { ObservableQuery } from './ObservableQuery'; import { NetworkStatus, isNetworkRequestInFlight } from './networkStatus'; @@ -1041,7 +1041,7 @@ export class QueryManager { optimistic = false, removeOptimistic = optimistic ? makeUniqueId("refetchQueries") : void 0, onQueryUpdated, - }: PrivateRefetchQueriesOptions>) { + }: InternalRefetchQueriesOptions>) { const includedQueriesById = new Map(); const results = new Map, any>(); diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index ac8c1cccfcc..7d4606c50f3 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -191,7 +191,9 @@ export interface SubscriptionOptions; -export interface PublicRefetchQueriesOptions< +// Used by ApolloClient["refetchQueries"] +// TODO Improve documentation comments for this public type. +export interface RefetchQueriesOptions< TData, TCache extends ApolloCache, > { @@ -205,10 +207,11 @@ export interface PublicRefetchQueriesOptions< onQueryUpdated?: OnQueryUpdated; } -export interface PrivateRefetchQueriesOptions< +// Used by QueryManager["refetchQueries"] +export interface InternalRefetchQueriesOptions< TData, TCache extends ApolloCache, -> extends Omit, "include"> { +> extends Omit, "include"> { // Just like the refetchQueries array for a mutation, allowing both strings // and PureQueryOptions objects. include?: RefetchQueryDescription; From 929feab237bb933de7244425060fa0926922b13b Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 20 Apr 2021 13:23:18 -0400 Subject: [PATCH 21/47] Rename cache.batch options.transaction function to options.update. I've never felt like "transaction" was exactly the right word for what cache.performTransaction and cache.batch are doing. For example, the updates are not automatically undone on failure like the word "transaction" suggests, unless you're recording against an optimistic layer that you plan to remove later, via cache.recordOptimisticTransaction and cache.removeOptimistic. In reality, the primary purpose of cache.performTransaction and cache.batch is to batch up broadcastWatches notifications for cache updates that take place within the callback function. I chose options.update because I think it's generic enough to avoid giving any mistaken interpretations of the purpose or usage of the function, unlike options.transaction. Separately, I believe options.update better aligns with the naming of the options.updateCache function for client.refetchQueries. Which may lead you to ask... Why not call it updateCache, exactly like client.refetchQueries? This is certainly a matter of subjective taste, but to me it feels redundant (and not useful for readability) to have to type the extra -Cache suffix every time you call cache.batch (which is more clearly a cache-updating method than client.refetchQueries is): cache.batch({ updateCache(cache) {...}, ... }) This commit will allow the following code, which eliminates the redundancy without sacrificing readability: cache.batch({ update(cache) {...}, ... }) For comparison, client.refetchQueries needs the extra specificity of updateCache because it gives readers of the code a clue about what's being updated (namely, the cache): client.refetchQueries({ updateCache(cache) {...}, ... }) --- src/cache/core/cache.ts | 2 +- src/cache/core/types/Cache.ts | 2 +- src/cache/inmemory/__tests__/cache.ts | 12 ++++---- src/cache/inmemory/inMemoryCache.ts | 43 +++++++++++++-------------- src/core/QueryManager.ts | 8 ++--- src/core/__tests__/ObservableQuery.ts | 2 +- 6 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/cache/core/cache.ts b/src/cache/core/cache.ts index 441d49a1ef0..db5c63879a8 100644 --- a/src/cache/core/cache.ts +++ b/src/cache/core/cache.ts @@ -63,7 +63,7 @@ export abstract class ApolloCache implements DataProxy { const optimisticId = typeof options.optimistic === "string" ? options.optimistic : options.optimistic === false ? null : void 0; - this.performTransaction(options.transaction, optimisticId); + this.performTransaction(options.update, optimisticId); } public abstract performTransaction( diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts index b9a1b713cb0..e9ff92021c4 100644 --- a/src/cache/core/types/Cache.ts +++ b/src/cache/core/types/Cache.ts @@ -57,7 +57,7 @@ export namespace Cache { export interface BatchOptions> { // Same as the first parameter of performTransaction, except the cache // argument will have the subclass type rather than ApolloCache. - transaction(cache: C): void; + update(cache: C): void; // Passing a string for this option creates a new optimistic layer, with the // given string as its layer.id, just like passing a string for the diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts index 6751b46fd8f..1db6fc073c6 100644 --- a/src/cache/inmemory/__tests__/cache.ts +++ b/src/cache/inmemory/__tests__/cache.ts @@ -1362,7 +1362,7 @@ describe('Cache', () => { const dirtied = new Map>(); cache.batch({ - transaction(cache) { + update(cache) { cache.writeQuery({ query: aQuery, data: { @@ -1403,7 +1403,7 @@ describe('Cache', () => { dirtied.clear(); cache.batch({ - transaction(cache) { + update(cache) { cache.writeQuery({ query: bQuery, data: { @@ -1474,7 +1474,7 @@ describe('Cache', () => { const dirtied = new Map>(); cache.batch({ - transaction(cache) { + update(cache) { cache.modify({ fields: { a(value, { INVALIDATE }) { @@ -1548,7 +1548,7 @@ describe('Cache', () => { const dirtied = new Map>(); cache.batch({ - transaction(cache) { + update(cache) { cache.modify({ fields: { a(value) { @@ -1571,7 +1571,7 @@ describe('Cache', () => { expect(aInfo.diffs).toEqual([ // This diff resulted from the cache.modify call in the cache.batch - // transaction function. + // update function. { complete: true, result: { @@ -1582,7 +1582,7 @@ describe('Cache', () => { expect(abInfo.diffs).toEqual([ // This diff resulted from the cache.modify call in the cache.batch - // transaction function. + // update function. { complete: true, result: { diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 660c9bb22de..a154204250d 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -341,7 +341,7 @@ export class InMemoryCache extends ApolloCache { public batch(options: Cache.BatchOptions) { const { - transaction, + update, optimistic = true, removeOptimistic, onWatchUpdated, @@ -354,7 +354,7 @@ export class InMemoryCache extends ApolloCache { this.data = this.optimisticData = layer; } try { - transaction(this); + update(this); } finally { --this.txCount; this.data = data; @@ -365,16 +365,15 @@ export class InMemoryCache extends ApolloCache { const alreadyDirty = new Set(); if (onWatchUpdated && !this.txCount) { - // If an options.onWatchUpdated callback is provided, we want to - // call it with only the Cache.WatchOptions objects affected by - // options.transaction, but there might be dirty watchers already - // waiting to be broadcast that have nothing to do with the - // transaction. To prevent including those watchers in the - // post-transaction broadcast, we perform this initial broadcast to - // collect the dirty watchers, so we can re-dirty them later, after - // the post-transaction broadcast, allowing them to receive their - // pending broadcasts the next time broadcastWatches is called, just - // as they would if we never called cache.batch. + // If an options.onWatchUpdated callback is provided, we want to call it + // with only the Cache.WatchOptions objects affected by options.update, + // but there might be dirty watchers already waiting to be broadcast that + // have nothing to do with the update. To prevent including those watchers + // in the post-update broadcast, we perform this initial broadcast to + // collect the dirty watchers, so we can re-dirty them later, after the + // post-update broadcast, allowing them to receive their pending + // broadcasts the next time broadcastWatches is called, just as they would + // if we never called cache.batch. this.broadcastWatches({ ...options, onWatchUpdated(watch) { @@ -391,14 +390,14 @@ export class InMemoryCache extends ApolloCache { this.optimisticData = this.optimisticData.addLayer(optimistic, perform); } else if (optimistic === false) { // Ensure both this.data and this.optimisticData refer to the root - // (non-optimistic) layer of the cache during the transaction. Note - // that this.data could be a Layer if we are currently executing an - // optimistic transaction function, but otherwise will always be an - // EntityStore.Root instance. + // (non-optimistic) layer of the cache during the update. Note that + // this.data could be a Layer if we are currently executing an optimistic + // update function, but otherwise will always be an EntityStore.Root + // instance. perform(this.data); } else { - // Otherwise, leave this.data and this.optimisticData unchanged and - // run the transaction with broadcast batching. + // Otherwise, leave this.data and this.optimisticData unchanged and run + // the update with broadcast batching. perform(); } @@ -423,8 +422,8 @@ export class InMemoryCache extends ApolloCache { return result; } }); - // Silently re-dirty any watches that were already dirty before the - // transaction was performed, and were not broadcast just now. + // Silently re-dirty any watches that were already dirty before the update + // was performed, and were not broadcast just now. if (alreadyDirty.size) { alreadyDirty.forEach(watch => this.maybeBroadcastWatch.dirty(watch)); } @@ -437,11 +436,11 @@ export class InMemoryCache extends ApolloCache { } public performTransaction( - transaction: (cache: InMemoryCache) => any, + update: (cache: InMemoryCache) => any, optimisticId?: string | null, ) { return this.batch({ - transaction, + update, optimistic: optimisticId || (optimisticId !== null), }); } diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index a10e044878f..b7c694156ae 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1095,11 +1095,11 @@ export class QueryManager { if (updateCache) { this.cache.batch({ - transaction: updateCache, + update: updateCache, // Since you can perform any combination of cache reads and/or writes in - // the cache.batch transaction function, its optimistic option can be - // either a boolean or a string, representing three distinct modes of + // the cache.batch update function, its optimistic option can be either + // a boolean or a string, representing three distinct modes of // operation: // // * false: read/write only the root layer @@ -1110,7 +1110,7 @@ export class QueryManager { // temporarily created within cache.batch with that string as its ID. If // we then pass that same string as the removeOptimistic option, we can // make cache.batch immediately remove the optimistic layer after - // running the transaction, triggering only one broadcast. + // running the updateCache function, triggering only one broadcast. // // However, the refetchQueries method accepts only true or false for its // optimistic option (not string). We interpret true to mean a temporary diff --git a/src/core/__tests__/ObservableQuery.ts b/src/core/__tests__/ObservableQuery.ts index 5dc2903cc49..7caa643e226 100644 --- a/src/core/__tests__/ObservableQuery.ts +++ b/src/core/__tests__/ObservableQuery.ts @@ -1957,7 +1957,7 @@ describe('ObservableQuery', () => { cache.batch({ optimistic: true, - transaction(cache) { + update(cache) { cache.modify({ fields: { people_one(value, { INVALIDATE }) { From 8a52c589262181c37b36af624d247bf44182fe22 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 21 Apr 2021 13:31:46 -0400 Subject: [PATCH 22/47] Make OnQueryUpdated lastDiff parameter non-optional. --- src/core/QueryManager.ts | 27 +++++++++++++++++---------- src/core/types.ts | 2 +- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index b7c694156ae..5afbbfe0c9c 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1042,9 +1042,12 @@ export class QueryManager { removeOptimistic = optimistic ? makeUniqueId("refetchQueries") : void 0, onQueryUpdated, }: InternalRefetchQueriesOptions>) { - const includedQueriesById = new Map(); - const results = new Map, any>(); + const includedQueriesById = new Map | undefined; + }>(); + const results = new Map, any>(); function maybeAddResult(oq: ObservableQuery, result: any): boolean { // The onQueryUpdated function can return false to ignore this query and // skip its normal broadcast, or true to allow the usual broadcast to @@ -1076,7 +1079,10 @@ export class QueryManager { if (typeof queryNameOrOptions === "string") { const queryId = queryIdsByQueryName[queryNameOrOptions]; if (queryId) { - includedQueriesById.set(queryId, queryNameOrOptions); + includedQueriesById.set(queryId, { + desc: queryNameOrOptions, + diff: this.getQuery(queryId).getDiff(), + }); } else { invariant.warn(`Unknown query name ${ JSON.stringify(queryNameOrOptions) @@ -1087,7 +1093,8 @@ export class QueryManager { // We will be issuing a fresh network request for this query, so we // pre-allocate a new query ID here. this.generateQueryId(), - queryNameOrOptions, + { desc: queryNameOrOptions, + diff: void 0 }, ); } }); @@ -1151,23 +1158,23 @@ export class QueryManager { } if (includedQueriesById.size) { - includedQueriesById.forEach((queryNameOrOptions, queryId) => { + includedQueriesById.forEach(({ desc, diff }, queryId) => { const queryInfo = this.getQuery(queryId); let oq = queryInfo.observableQuery; if (oq) { maybeAddResult( oq, onQueryUpdated - ? onQueryUpdated(oq, queryInfo.getDiff()) + ? onQueryUpdated(oq, queryInfo.getDiff(), diff) : oq.refetch(), ); - } else if (typeof queryNameOrOptions === "object") { + } else if (typeof desc === "object") { const options: WatchQueryOptions = { - query: queryNameOrOptions.query, - variables: queryNameOrOptions.variables, + query: desc.query, + variables: desc.variables, fetchPolicy: "network-only", - context: queryNameOrOptions.context, + context: desc.context, }; queryInfo.setObservableQuery(oq = new ObservableQuery({ diff --git a/src/core/types.ts b/src/core/types.ts index e849fa85875..9fa0c0c48ee 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -18,7 +18,7 @@ export type QueryListener = (queryInfo: QueryInfo) => void; export type OnQueryUpdated = ( observableQuery: ObservableQuery, diff: Cache.DiffResult, - lastDiff?: Cache.DiffResult, + lastDiff: Cache.DiffResult | undefined, ) => boolean | Promise>; export type OperationVariables = Record; From 3439fd677e5f81741abf1f8ee238aa8f119888f5 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 21 Apr 2021 13:52:17 -0400 Subject: [PATCH 23/47] Improve/simplify refetchQueries options.include processing. --- src/core/QueryManager.ts | 116 ++++++++++++++++++---------------- src/core/watchQueryOptions.ts | 3 +- 2 files changed, 64 insertions(+), 55 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 5afbbfe0c9c..dac7afa42a8 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -30,6 +30,7 @@ import { ErrorPolicy, RefetchQueryDescription, InternalRefetchQueriesOptions, + RefetchQueryDescriptor, } from './watchQueryOptions'; import { ObservableQuery } from './ObservableQuery'; import { NetworkStatus, isNetworkRequestInFlight } from './networkStatus'; @@ -579,6 +580,7 @@ export class QueryManager { public query( options: QueryOptions, + queryId = this.generateQueryId(), ): Promise> { invariant( options.query, @@ -601,7 +603,6 @@ export class QueryManager { 'pollInterval option only supported on watchQuery.', ); - const queryId = this.generateQueryId(); return this.fetchQuery( queryId, options, @@ -1043,10 +1044,23 @@ export class QueryManager { onQueryUpdated, }: InternalRefetchQueriesOptions>) { const includedQueriesById = new Map | undefined; }>(); + if (include) { + include.forEach(desc => { + getQueryIdsForQueryDescriptor(this, desc).forEach(queryId => { + includedQueriesById.set(queryId, { + desc, + diff: typeof desc === "string" + ? this.getQuery(queryId).getDiff() + : void 0, + }); + }); + }); + } + const results = new Map, any>(); function maybeAddResult(oq: ObservableQuery, result: any): boolean { // The onQueryUpdated function can return false to ignore this query and @@ -1065,41 +1079,6 @@ export class QueryManager { return false; } - if (include) { - const queryIdsByQueryName: Record = Object.create(null); - this.queries.forEach((queryInfo, queryId) => { - const oq = queryInfo.observableQuery; - const queryName = oq && oq.queryName; - if (queryName) { - queryIdsByQueryName[queryName] = queryId; - } - }); - - include.forEach(queryNameOrOptions => { - if (typeof queryNameOrOptions === "string") { - const queryId = queryIdsByQueryName[queryNameOrOptions]; - if (queryId) { - includedQueriesById.set(queryId, { - desc: queryNameOrOptions, - diff: this.getQuery(queryId).getDiff(), - }); - } else { - invariant.warn(`Unknown query name ${ - JSON.stringify(queryNameOrOptions) - } passed to refetchQueries method in options.include array`); - } - } else { - includedQueriesById.set( - // We will be issuing a fresh network request for this query, so we - // pre-allocate a new query ID here. - this.generateQueryId(), - { desc: queryNameOrOptions, - diff: void 0 }, - ); - } - }); - } - if (updateCache) { this.cache.batch({ update: updateCache, @@ -1161,21 +1140,15 @@ export class QueryManager { includedQueriesById.forEach(({ desc, diff }, queryId) => { const queryInfo = this.getQuery(queryId); let oq = queryInfo.observableQuery; - if (oq) { - maybeAddResult( - oq, - onQueryUpdated - ? onQueryUpdated(oq, queryInfo.getDiff(), diff) - : oq.refetch(), - ); + let fallback: undefined | (() => Promise>); - } else if (typeof desc === "object") { - const options: WatchQueryOptions = { - query: desc.query, - variables: desc.variables, + if (typeof desc === "string") { + fallback = () => oq!.refetch(); + } else if (desc && typeof desc === "object") { + const options = { + ...desc, fetchPolicy: "network-only", - context: desc.context, - }; + } as QueryOptions; queryInfo.setObservableQuery(oq = new ObservableQuery({ queryManager: this, @@ -1183,10 +1156,19 @@ export class QueryManager { options, })); - const fetchPromise = this.fetchQuery(queryId, options); - maybeAddResult(oq, fetchPromise); - const stop = () => this.stopQuery(queryId); - fetchPromise.then(stop, stop); + fallback = () => this.query(options, queryId); + } + + if (oq && fallback) { + maybeAddResult( + oq, + // 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. + onQueryUpdated + ? onQueryUpdated(oq, queryInfo.getDiff(), diff) + : fallback(), + ); } }); } @@ -1375,6 +1357,32 @@ export class QueryManager { } } +function getQueryIdsForQueryDescriptor( + qm: QueryManager, + desc: RefetchQueryDescriptor, +) { + const queryIds: string[] = []; + if (typeof desc === "string") { + qm["queries"].forEach(({ observableQuery: oq }, queryId) => { + if (oq && + oq.queryName === desc && + 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; +} + const prefixCounts: Record = Object.create(null); function makeUniqueId(prefix: string) { const count = prefixCounts[prefix] || 1; diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index 7d4606c50f3..32b20a3ac71 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -189,7 +189,8 @@ export interface SubscriptionOptions; +export type RefetchQueryDescriptor = string | PureQueryOptions; +export type RefetchQueryDescription = RefetchQueryDescriptor[]; // Used by ApolloClient["refetchQueries"] // TODO Improve documentation comments for this public type. From c9d0428b617f5fb7f5f4fdde4896f9372633924c Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 21 Apr 2021 19:00:32 -0400 Subject: [PATCH 24/47] Make client.refetchQueries(...) awaitable, and rename results array. --- src/core/ApolloClient.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index 7bf6185b84f..f461257eff3 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -540,20 +540,30 @@ export class ApolloClient implements DataProxy { TCache extends ApolloCache = ApolloCache, >( options: RefetchQueriesOptions, - ): { + ): PromiseLike[]> & { queries: ObservableQuery[]; - updates: any[]; + results: any[]; } { const map = this.queryManager.refetchQueries(options); const queries: ObservableQuery[] = []; - const updates: any[] = []; + const results: any[] = []; - map.forEach((result, obsQuery) => { + map.forEach((update, obsQuery) => { queries.push(obsQuery); - updates.push(result); + results.push(update); }); - return { queries, updates }; + return { + // In case you need the raw results immediately, without awaiting + // Promise.all(results): + queries, + results, + // Using a thenable instead of a Promise here allows us to avoid creating + // the Promise if it isn't going to be awaited. + then(onResolved, onRejected) { + return Promise.all(results).then(onResolved, onRejected); + }, + }; } /** From 6bfa317e39c0f76d4b47462ddefc78e450f282e9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 22 Apr 2021 14:26:58 -0400 Subject: [PATCH 25/47] Call fallback() if onQueryUpdated returns true for an included query. --- src/core/QueryManager.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index dac7afa42a8..cb4c87dacd8 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1160,15 +1160,15 @@ export class QueryManager { } if (oq && fallback) { - maybeAddResult( - oq, - // 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. - onQueryUpdated - ? onQueryUpdated(oq, queryInfo.getDiff(), diff) - : fallback(), - ); + // 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. + let result = onQueryUpdated && + onQueryUpdated(oq, queryInfo.getDiff(), diff); + if (!onQueryUpdated || result === true) { + result = fallback(); + } + maybeAddResult(oq, result); } }); } From 0e9d5a5a125cadeb7c78d911571e6f7aa5222fc8 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 11 May 2021 17:12:46 -0400 Subject: [PATCH 26/47] Allow onQueryUpdated functions to return anything. The true and false return values still have their special meanings, but anything else goes (not just Promise>). The type of whatever you return should be inferred by TypeScript, so the results array you get by awaiting client.refetchQueries will have the correct type: client.refetchQueries({ ... onQueryUpdated() { return Promise.resolve("value"); } }).then(results => { // TypeScript knows results is a string[], not a Promise[] }); I also removed the TData type parameter from the OnQueryUpdated function type, since there's not usually enough type information for TypeScript to figure out what TData is. --- src/core/ApolloClient.ts | 9 +++++---- src/core/QueryManager.ts | 8 ++++---- src/core/types.ts | 13 ++++++++----- src/core/watchQueryOptions.ts | 10 +++++----- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index f461257eff3..8f5b79164f3 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -14,6 +14,7 @@ import { ApolloQueryResult, DefaultContext, OperationVariables, + PromiseResult, Resolvers, } from './types'; @@ -536,13 +537,13 @@ export class ApolloClient implements DataProxy { * Takes optional parameter `includeStandby` which will include queries in standby-mode when refetching. */ public refetchQueries< - TData, TCache extends ApolloCache = ApolloCache, + TResult = any, >( - options: RefetchQueriesOptions, - ): PromiseLike[]> & { + options: RefetchQueriesOptions, + ): PromiseLike[]> & { queries: ObservableQuery[]; - results: any[]; + results: TResult[]; } { const map = this.queryManager.refetchQueries(options); const queries: ObservableQuery[] = []; diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index cb4c87dacd8..579b333e4fb 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -330,7 +330,7 @@ export class QueryManager { update?: MutationUpdaterFunction; refetchQueries?: RefetchQueryDescription; removeOptimistic?: string; - onQueryUpdated?: OnQueryUpdated; + onQueryUpdated?: OnQueryUpdated; }, cache = this.cache, ): Promise { @@ -1036,13 +1036,13 @@ export class QueryManager { return concast; } - public refetchQueries({ + public refetchQueries({ updateCache, include, optimistic = false, removeOptimistic = optimistic ? makeUniqueId("refetchQueries") : void 0, onQueryUpdated, - }: InternalRefetchQueriesOptions>) { + }: InternalRefetchQueriesOptions, TResult>) { const includedQueriesById = new Map | undefined; @@ -1140,7 +1140,7 @@ export class QueryManager { includedQueriesById.forEach(({ desc, diff }, queryId) => { const queryInfo = this.getQuery(queryId); let oq = queryInfo.observableQuery; - let fallback: undefined | (() => Promise>); + let fallback: undefined | (() => any); if (typeof desc === "string") { fallback = () => oq!.refetch(); diff --git a/src/core/types.ts b/src/core/types.ts index 9fa0c0c48ee..5be464545e1 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -15,11 +15,14 @@ export type DefaultContext = Record; export type QueryListener = (queryInfo: QueryInfo) => void; -export type OnQueryUpdated = ( - observableQuery: ObservableQuery, - diff: Cache.DiffResult, - lastDiff: Cache.DiffResult | undefined, -) => boolean | Promise>; +export type OnQueryUpdated = ( + observableQuery: ObservableQuery, + diff: Cache.DiffResult, + lastDiff: Cache.DiffResult | undefined, +) => boolean | TResult; + +export type PromiseResult = + T extends PromiseLike ? U : T; export type OperationVariables = Record; diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index 32b20a3ac71..c2e217489a7 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -195,8 +195,8 @@ export type RefetchQueryDescription = RefetchQueryDescriptor[]; // Used by ApolloClient["refetchQueries"] // TODO Improve documentation comments for this public type. export interface RefetchQueriesOptions< - TData, TCache extends ApolloCache, + TResult, > { updateCache?: (cache: TCache) => void; // Although you can pass PureQueryOptions objects in addition to strings in @@ -205,14 +205,14 @@ export interface RefetchQueriesOptions< // public type of the options.include array to string[] (just query names). include?: string[]; optimistic?: boolean; - onQueryUpdated?: OnQueryUpdated; + onQueryUpdated?: OnQueryUpdated; } // Used by QueryManager["refetchQueries"] export interface InternalRefetchQueriesOptions< - TData, TCache extends ApolloCache, -> extends Omit, "include"> { + TResult, +> extends Omit, "include"> { // Just like the refetchQueries array for a mutation, allowing both strings // and PureQueryOptions objects. include?: RefetchQueryDescription; @@ -290,7 +290,7 @@ export interface MutationBaseOptions< * A function that will be called for each ObservableQuery affected by * this mutation, after the mutation has completed. */ - onQueryUpdated?: OnQueryUpdated; + onQueryUpdated?: OnQueryUpdated; /** * Specifies the {@link ErrorPolicy} to be used for this operation From 6bc76acad5223d176449943d17c0ccfff2cc12af Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 11 May 2021 17:48:29 -0400 Subject: [PATCH 27/47] Make explicitly-included refetchQueries queries (re)read from cache. --- src/core/QueryManager.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 579b333e4fb..1183f35902b 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1160,11 +1160,14 @@ export class QueryManager { } if (oq && fallback) { + let result: boolean | TResult | undefined; // 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. - let result = onQueryUpdated && - onQueryUpdated(oq, queryInfo.getDiff(), diff); + if (onQueryUpdated) { + queryInfo.reset(); // Force queryInfo.getDiff() to read from cache. + result = onQueryUpdated(oq, queryInfo.getDiff(), diff); + } if (!onQueryUpdated || result === true) { result = fallback(); } From fd8a94c414e58a781ce12ea8a2c7a566661ddebe Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 11 May 2021 15:01:04 -0400 Subject: [PATCH 28/47] Basic tests of client.refetchQueries API. --- src/__tests__/refetchQueries.ts | 256 ++++++++++++++++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 src/__tests__/refetchQueries.ts diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts new file mode 100644 index 00000000000..1c8081ba1eb --- /dev/null +++ b/src/__tests__/refetchQueries.ts @@ -0,0 +1,256 @@ +import { Subscription } from "zen-observable-ts"; + +import { itAsync } from '../utilities/testing/itAsync'; +import { + ApolloClient, + ApolloLink, + InMemoryCache, + gql, + Observable, + TypedDocumentNode, + ObservableQuery, +} from "../core"; + +describe("client.refetchQueries", () => { + itAsync("is public and callable", (resolve, reject) => { + const client = new ApolloClient({ + cache: new InMemoryCache, + }); + expect(typeof client.refetchQueries).toBe("function"); + + const result = client.refetchQueries({ + updateCache(cache) { + expect(cache).toBe(client.cache); + expect(cache.extract()).toEqual({}); + }, + onQueryUpdated(obsQuery, diff) { + reject("should not have called onQueryUpdated"); + return false; + }, + }); + + expect(result.queries).toEqual([]); + expect(result.results).toEqual([]); + + result.then(resolve, reject); + }); + + const aQuery: TypedDocumentNode<{ a: string }> = gql`query A { a }`; + const bQuery: TypedDocumentNode<{ b: string }> = gql`query B { b }`; + const abQuery: TypedDocumentNode<{ + a: string; + b: string; + }> = gql`query AB { a b }`; + + function makeClient() { + return new ApolloClient({ + cache: new InMemoryCache, + link: new ApolloLink(operation => new Observable(observer => { + const data: Record = {}; + operation.operationName.split("").forEach(letter => { + data[letter.toLowerCase()] = letter.toUpperCase(); + }); + observer.next({ data }); + observer.complete(); + })), + }); + } + + const subs: Subscription[] = []; + function unsubscribe() { + subs.splice(0).forEach(sub => sub.unsubscribe()); + } + + function setup(client = makeClient()) { + function watch(query: TypedDocumentNode) { + const obsQuery = client.watchQuery({ query }); + return new Promise>((resolve, reject) => { + subs.push(obsQuery.subscribe({ + error: reject, + next(result) { + expect(result.loading).toBe(false); + resolve(obsQuery); + }, + })); + }); + } + + return Promise.all([ + watch(aQuery), + watch(bQuery), + watch(abQuery), + ]); + } + + // Not a great way to sort objects, but it will give us stable orderings in + // these specific tests (especially since the keys are all "a" and/or "b"). + function sortObjects(array: T) { + array.sort((a, b) => { + const aKey = Object.keys(a).join(","); + const bKey = Object.keys(b).join(","); + if (aKey < bKey) return -1; + if (bKey < aKey) return 1; + return 0; + }); + } + + itAsync("includes watched queries affected by updateCache", async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const ayyResults = await client.refetchQueries({ + 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) { + reject("bQuery should not have been updated"); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "Ayy", b: "B" }); + } else { + reject("unexpected ObservableQuery"); + } + return Promise.resolve(diff.result); + }, + }); + + sortObjects(ayyResults); + + expect(ayyResults).toEqual([ + { a: "Ayy" }, + { a: "Ayy", b: "B" }, + // Note that no bQuery result is included here. + ]); + + const beeResults = await client.refetchQueries({ + updateCache(cache) { + cache.writeQuery({ + query: bQuery, + data: { + b: "Bee", + }, + }); + }, + + onQueryUpdated(obs, diff) { + if (obs === aObs) { + reject("aQuery should not have been updated"); + } 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"); + } + return diff.result; + }, + }); + + sortObjects(beeResults); + + expect(beeResults).toEqual([ + // Note that no aQuery result is included here. + { a: "Ayy", b: "Bee" }, + { b: "Bee" }, + ]); + + unsubscribe(); + resolve(); + }); + + itAsync("includes watched queries named in options.include", async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const ayyResults = await client.refetchQueries({ + updateCache(cache) { + cache.writeQuery({ + query: aQuery, + data: { + a: "Ayy", + }, + }); + }, + + // This is the options.include array mentioned in the test description. + include: ["B"], + + 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"); + } + return Promise.resolve(diff.result); + }, + }); + + sortObjects(ayyResults); + + expect(ayyResults).toEqual([ + { a: "Ayy" }, + { a: "Ayy", b: "B" }, + // Included this time! + { b: "B" }, + ]); + + const beeResults = await client.refetchQueries({ + updateCache(cache) { + cache.writeQuery({ + query: bQuery, + data: { + b: "Bee", + }, + }); + }, + + // The "A" here causes aObs to be included, but the "AB" should be + // redundant because that query is already included. + include: ["A", "AB"], + + 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"); + } + return diff.result; + }, + }); + + sortObjects(beeResults); + + expect(beeResults).toEqual([ + { a: "Ayy" }, // Included this time! + { a: "Ayy", b: "Bee" }, + { b: "Bee" }, + ]); + + unsubscribe(); + resolve(); + }); +}); From e02f8a11f807686873d3a3ba269b84041829e5e7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 11 May 2021 19:42:14 -0400 Subject: [PATCH 29/47] Refetch updateCache-triggered queries if onQueryUpdated not provided. --- src/__tests__/refetchQueries.ts | 45 +++++++++++++++++++++++++++++++++ src/core/ApolloClient.ts | 2 +- src/core/QueryManager.ts | 21 ++++++++++++--- src/core/watchQueryOptions.ts | 7 ++++- 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index 1c8081ba1eb..eef4723002e 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -253,4 +253,49 @@ describe("client.refetchQueries", () => { unsubscribe(); resolve(); }); + + itAsync("refetches watched queries if onQueryUpdated not provided", async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const aSpy = jest.spyOn(aObs, "refetch"); + const bSpy = jest.spyOn(bObs, "refetch"); + const abSpy = jest.spyOn(abObs, "refetch"); + + const ayyResults = ( + await client.refetchQueries({ + include: ["B"], + updateCache(cache) { + cache.writeQuery({ + query: aQuery, + data: { + a: "Ayy", + }, + }); + }, + }) + ).map(result => result.data as object); + + sortObjects(ayyResults); + + // These results have reverted back to what the ApolloLink returns ("A" + // rather than "Ayy"), because we let them be refetched (by not providing + // an onQueryUpdated function). + expect(ayyResults).toEqual([ + { a: "A" }, + { a: "A", b: "B" }, + { b: "B" }, + ]); + + expect(aSpy).toHaveBeenCalledTimes(1); + expect(bSpy).toHaveBeenCalledTimes(1); + expect(abSpy).toHaveBeenCalledTimes(1); + + unsubscribe(); + resolve(); + }); }); diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index 8f5b79164f3..ae8b27705b4 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -538,7 +538,7 @@ export class ApolloClient implements DataProxy { */ public refetchQueries< TCache extends ApolloCache = ApolloCache, - TResult = any, + TResult = Promise>, >( options: RefetchQueriesOptions, ): PromiseLike[]> & { diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 1183f35902b..5462fb12d1f 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -410,7 +410,9 @@ export class QueryManager { // Let the caller of client.mutate optionally determine the refetching // behavior for watched queries after the mutation.update function runs. - onQueryUpdated: mutation.onQueryUpdated, + // If no onQueryUpdated function was provided for this mutation, pass + // null instead of undefined to disable the default refetching behavior. + onQueryUpdated: mutation.onQueryUpdated || null, }).forEach(result => results.push(result)); @@ -1123,14 +1125,25 @@ export class QueryManager { // temporary optimistic layer, in case that ever matters. removeOptimistic, - onWatchUpdated: onQueryUpdated && function (watch, diff, lastDiff) { + onWatchUpdated(watch, diff, lastDiff) { const oq = watch.watcher instanceof QueryInfo && watch.watcher.observableQuery; if (oq) { - includedQueriesById.delete(oq.queryId); - return maybeAddResult(oq, onQueryUpdated(oq, diff, lastDiff)); + if (onQueryUpdated) { + includedQueriesById.delete(oq.queryId); + return maybeAddResult(oq, onQueryUpdated(oq, diff, lastDiff)); + } + if (onQueryUpdated !== null) { + // 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 || ``, + diff, + }); + } } }, }); diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index c2e217489a7..4543abcb44e 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -205,7 +205,12 @@ export interface RefetchQueriesOptions< // public type of the options.include array to string[] (just query names). include?: string[]; optimistic?: boolean; - onQueryUpdated?: OnQueryUpdated; + // If no onQueryUpdated function is provided, any queries affected by the + // updateCache function or included in the options.include array will be + // refetched by default. Passing null instead of undefined disables this + // default refetching behavior for affected queries, though included queries + // will still be refetched. + onQueryUpdated?: OnQueryUpdated | null; } // Used by QueryManager["refetchQueries"] From 967ade34b3bc096f844ca907344456057075f272 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 12 May 2021 11:13:29 -0400 Subject: [PATCH 30/47] Add a few TODOs to client.refetchQueries test file. --- src/__tests__/refetchQueries.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index eef4723002e..e30d7cd8efd 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -298,4 +298,20 @@ describe("client.refetchQueries", () => { unsubscribe(); resolve(); }); + + it("can run updateQuery function against optimistic cache layer", () => { + // TODO + }); + + it("can return true from onQueryUpdated to choose default refetching behavior", () => { + // TODO + }); + + it("can return false from onQueryUpdated to skip the updated query", () => { + // TODO + }); + + it("can refetch no-cache queries", () => { + // TODO + }); }); From dfe9e684d3bdebc6445a68a54ed2bfa0f36ebb07 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 12 Jan 2021 14:34:16 -0500 Subject: [PATCH 31/47] Make dirtying of removed EntityStore Layer fields more precise. Previously, removing an optimistic EntityStore layer with removeOptimistic would dirty all fields of any StoreObjects contained by the removed layer, ignoring the possibility that some of those field values might have been inherited as-is from the parent layer. With this commit, we dirty only those fields whose values will be observably changed by removing the layer, which requires comparing field values between the layer-to-be-removed and its parent layer. --- src/cache/inmemory/entityStore.ts | 38 ++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index 5bf4792e026..82afc70a73d 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -642,18 +642,44 @@ class Layer extends EntityStore { const parent = this.parent.removeLayer(layerId); if (layerId === this.id) { - // Dirty every ID we're removing. if (this.group.caching) { + // Dirty every ID we're removing. Technically we might be able to avoid + // dirtying fields that have values in higher layers, but we don't have + // easy access to higher layers here, and we're about to recreate those + // layers anyway (see parent.addLayer below). Object.keys(this.data).forEach(dataId => { - // If this.data[dataId] contains nothing different from what - // lies beneath, we can avoid dirtying this dataId and all of - // its fields, and simply discard this Layer. The only reason we - // call this.delete here is to dirty the removed fields. - if (this.data[dataId] !== (parent as Layer).lookup(dataId)) { + const ownStoreObject = this.data[dataId]; + const parentStoreObject = parent["lookup"](dataId); + if (!parentStoreObject) { + // The StoreObject identified by dataId was defined in this layer + // but will be undefined in the parent layer, so we can delete the + // whole entity using this.delete(dataId). Since we're about to + // throw this layer away, the only goal of this deletion is to dirty + // the removed fields. this.delete(dataId); + } else if (!ownStoreObject) { + // This layer had an entry for dataId but it was undefined, which + // means the entity was deleted in this layer, and it's about to + // become undeleted when we remove this layer, so we need to dirty + // all fields that are about to be reexposed. + this.group.dirty(dataId, "__exists"); + Object.keys(parentStoreObject).forEach(storeFieldName => { + this.group.dirty(dataId, storeFieldName); + }); + } else if (ownStoreObject !== parentStoreObject) { + // If ownStoreObject is not exactly the same as parentStoreObject, + // dirty any fields whose values will change as a result of this + // removal. + Object.keys(ownStoreObject).forEach(storeFieldName => { + if (!equal(ownStoreObject[storeFieldName], + parentStoreObject[storeFieldName])) { + this.group.dirty(dataId, storeFieldName); + } + }); } }); } + return parent; } From bee43b0ccb6791e985e907391c373ba67ea00a88 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 12 May 2021 18:08:45 -0400 Subject: [PATCH 32/47] Test client.refetchQueries({ optimistic: true }). --- src/__tests__/refetchQueries.ts | 80 +++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index e30d7cd8efd..41474f2d409 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -23,7 +23,7 @@ describe("client.refetchQueries", () => { expect(cache).toBe(client.cache); expect(cache.extract()).toEqual({}); }, - onQueryUpdated(obsQuery, diff) { + onQueryUpdated() { reject("should not have called onQueryUpdated"); return false; }, @@ -299,8 +299,82 @@ describe("client.refetchQueries", () => { resolve(); }); - it("can run updateQuery function against optimistic cache layer", () => { - // TODO + itAsync("can run updateQuery function against optimistic cache layer", async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + client.cache.watch({ + query: abQuery, + optimistic: false, + callback(diff) { + reject("should not have notified non-optimistic watcher"); + }, + }); + + expect(client.cache.extract(true)).toEqual({ + ROOT_QUERY: { + __typename: "Query", + "a": "A", + "b": "B", + }, + }); + + const results = await client.refetchQueries({ + // This causes the update to run against a temporary optimistic layer. + optimistic: true, + + updateCache(cache) { + const modified = cache.modify({ + fields: { + a(value, { DELETE }) { + expect(value).toEqual("A"); + return DELETE; + }, + }, + }); + expect(modified).toBe(true); + }, + + onQueryUpdated(obs, diff) { + expect(diff.complete).toBe(true); + + // Even though we evicted the Query.a field in the updateCache function, + // that optimistic layer was discarded before broadcasting results, so + // we're back to the original (non-optimistic) data. + if (obs === aObs) { + expect(diff.result).toEqual({ a: "A" }); + } else if (obs === bObs) { + reject("bQuery should not have been updated"); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "A", b: "B" }); + } else { + reject("unexpected ObservableQuery"); + } + + return diff.result; + }, + }); + + sortObjects(results); + + expect(results).toEqual([ + { a: "A" }, + { a: "A", b: "B" }, + ]); + + expect(client.cache.extract(true)).toEqual({ + ROOT_QUERY: { + __typename: "Query", + "a": "A", + "b": "B", + }, + }); + + resolve(); }); it("can return true from onQueryUpdated to choose default refetching behavior", () => { From 70bd843e436917019cd0ccf936e8f037ecf56f5e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 14:42:11 -0400 Subject: [PATCH 33/47] Move refetchQueries-related types into src/core/types.ts. --- src/core/ApolloClient.ts | 2 +- src/core/QueryManager.ts | 6 +++--- src/core/types.ts | 37 +++++++++++++++++++++++++++++++++ src/core/watchQueryOptions.ts | 39 +---------------------------------- 4 files changed, 42 insertions(+), 42 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index ae8b27705b4..f19a044683b 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -16,6 +16,7 @@ import { OperationVariables, PromiseResult, Resolvers, + RefetchQueriesOptions, } from './types'; import { @@ -24,7 +25,6 @@ import { MutationOptions, SubscriptionOptions, WatchQueryFetchPolicy, - RefetchQueriesOptions, } from './watchQueryOptions'; import { diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 5462fb12d1f..a65955266ef 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -28,9 +28,6 @@ import { MutationOptions, WatchQueryFetchPolicy, ErrorPolicy, - RefetchQueryDescription, - InternalRefetchQueriesOptions, - RefetchQueryDescriptor, } from './watchQueryOptions'; import { ObservableQuery } from './ObservableQuery'; import { NetworkStatus, isNetworkRequestInFlight } from './networkStatus'; @@ -39,6 +36,9 @@ import { OperationVariables, MutationUpdaterFunction, OnQueryUpdated, + RefetchQueryDescription, + InternalRefetchQueriesOptions, + RefetchQueryDescriptor, } from './types'; import { LocalState } from './LocalState'; diff --git a/src/core/types.ts b/src/core/types.ts index 5be464545e1..6de3e58c404 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -24,6 +24,43 @@ export type OnQueryUpdated = ( export type PromiseResult = T extends PromiseLike ? U : T; +export type RefetchQueryDescriptor = string | PureQueryOptions; +export type RefetchQueryDescription = RefetchQueryDescriptor[]; + +// Used by ApolloClient["refetchQueries"] +// TODO Improve documentation comments for this public type. +export interface RefetchQueriesOptions< + TCache extends ApolloCache, + 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?: string[]; + optimistic?: boolean; + // If no onQueryUpdated function is provided, any queries affected by the + // updateCache function or included in the options.include array will be + // refetched by default. Passing null instead of undefined disables this + // default refetching behavior for affected queries, though included queries + // will still be refetched. + onQueryUpdated?: OnQueryUpdated | null; +} + +// Used by QueryManager["refetchQueries"] +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; + // This part of the API is a (useful) implementation detail, but need not be + // exposed in the public client.refetchQueries API (above). + removeOptimistic?: string; +} + export type OperationVariables = Record; export type PureQueryOptions = { diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index 4543abcb44e..ed958668540 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -5,10 +5,10 @@ import { FetchResult } from '../link/core'; import { DefaultContext, MutationQueryReducersMap, - PureQueryOptions, OperationVariables, MutationUpdaterFunction, OnQueryUpdated, + RefetchQueryDescription, } from './types'; import { ApolloCache } from '../cache'; @@ -189,43 +189,6 @@ export interface SubscriptionOptions, - 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?: string[]; - optimistic?: boolean; - // If no onQueryUpdated function is provided, any queries affected by the - // updateCache function or included in the options.include array will be - // refetched by default. Passing null instead of undefined disables this - // default refetching behavior for affected queries, though included queries - // will still be refetched. - onQueryUpdated?: OnQueryUpdated | null; -} - -// Used by QueryManager["refetchQueries"] -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; - // This part of the API is a (useful) implementation detail, but need not be - // exposed in the public client.refetchQueries API (above). - removeOptimistic?: string; -} - export interface MutationBaseOptions< TData = any, TVariables = OperationVariables, From b75bd7dd73c6f7753fbeca944dbd08dc63eafa5b Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 15:29:02 -0400 Subject: [PATCH 34/47] Infer better refetchQueries return type from onQueryUpdated return type. --- src/core/ApolloClient.ts | 26 ++++++---------- src/core/QueryManager.ts | 5 +-- src/core/types.ts | 46 ++++++++++++++++++++++++++-- src/utilities/index.ts | 2 ++ src/utilities/types/IsStrictlyAny.ts | 17 ++++++++++ 5 files changed, 74 insertions(+), 22 deletions(-) create mode 100644 src/utilities/types/IsStrictlyAny.ts diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index f19a044683b..a2ab7b299c3 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -14,9 +14,9 @@ import { ApolloQueryResult, DefaultContext, OperationVariables, - PromiseResult, Resolvers, RefetchQueriesOptions, + RefetchQueriesResult, } from './types'; import { @@ -541,30 +541,22 @@ export class ApolloClient implements DataProxy { TResult = Promise>, >( options: RefetchQueriesOptions, - ): PromiseLike[]> & { - queries: ObservableQuery[]; - results: TResult[]; - } { + ): RefetchQueriesResult { const map = this.queryManager.refetchQueries(options); const queries: ObservableQuery[] = []; - const results: any[] = []; + const results: TResult[] = []; map.forEach((update, obsQuery) => { queries.push(obsQuery); results.push(update); }); - return { - // In case you need the raw results immediately, without awaiting - // Promise.all(results): - queries, - results, - // Using a thenable instead of a Promise here allows us to avoid creating - // the Promise if it isn't going to be awaited. - then(onResolved, onRejected) { - return Promise.all(results).then(onResolved, onRejected); - }, - }; + const result = Promise.all(results) as RefetchQueriesResult; + // In case you need the raw results immediately, without awaiting + // Promise.all(results): + result.queries = queries; + result.results = results; + return result; } /** diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index a65955266ef..9a067905d1c 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1044,7 +1044,8 @@ export class QueryManager { optimistic = false, removeOptimistic = optimistic ? makeUniqueId("refetchQueries") : void 0, onQueryUpdated, - }: InternalRefetchQueriesOptions, TResult>) { + }: InternalRefetchQueriesOptions, TResult> + ): Map, TResult> { const includedQueriesById = new Map | undefined; @@ -1063,7 +1064,7 @@ export class QueryManager { }); } - const results = new Map, any>(); + const results = new Map, TResult>(); function maybeAddResult(oq: ObservableQuery, result: any): boolean { // The onQueryUpdated function can return false to ignore this query and // skip its normal broadcast, or true to allow the usual broadcast to diff --git a/src/core/types.ts b/src/core/types.ts index 6de3e58c404..15b9c004905 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -8,6 +8,7 @@ import { NetworkStatus } from './networkStatus'; import { Resolver } from './LocalState'; import { ObservableQuery } from './ObservableQuery'; import { Cache } from '../cache'; +import { IsStrictlyAny } from '../utilities'; export { TypedDocumentNode } from '@graphql-typed-document-node/core'; @@ -21,9 +22,6 @@ export type OnQueryUpdated = ( lastDiff: Cache.DiffResult | undefined, ) => boolean | TResult; -export type PromiseResult = - T extends PromiseLike ? U : T; - export type RefetchQueryDescriptor = string | PureQueryOptions; export type RefetchQueryDescription = RefetchQueryDescriptor[]; @@ -48,6 +46,48 @@ export interface RefetchQueriesOptions< onQueryUpdated?: OnQueryUpdated | null; } +// The client.refetchQueries method returns a thenable (PromiseLike) object +// whose result is an array of Promise.resolve'd TResult values, where TResult +// is whatever type the (optional) onQueryUpdated function returns. When no +// onQueryUpdated function is given, TResult defaults to ApolloQueryResult +// (thanks to default type parameters for client.refetchQueries). +export type RefetchQueriesPromiseResults = + // If onQueryUpdated returns any, all bets are off, so the results array must + // be a generic any[] array, which is much less confusing than the union type + // we get if we don't check for any. I hoped `any extends TResult` would do + // the trick here, instead of IsStrictlyAny, but you can see for yourself what + // fails in the refetchQueries tests if you try making that simplification. + IsStrictlyAny extends true ? any[] : + // If the onQueryUpdated function passed to client.refetchQueries returns true + // or false, that means either to refetch the query (true) or to skip the + // query (false). Since refetching produces an ApolloQueryResult, and + // skipping produces nothing, the fully-resolved array of all results produced + // will be an ApolloQueryResult[], when TResult extends boolean. + TResult extends boolean ? ApolloQueryResult[] : + // If onQueryUpdated returns a PromiseLike, that thenable will be passed as + // an array element to Promise.all, so we infer/unwrap the array type U here. + TResult extends PromiseLike ? U[] : + // All other onQueryUpdated results end up in the final Promise.all array as + // themselves, with their original TResult type. Note that TResult will + // default to ApolloQueryResult if no onQueryUpdated function is passed + // to client.refetchQueries. + TResult[]; + +export type RefetchQueriesResult = + // The result of client.refetchQueries is thenable/awaitable, if you just want + // an array of fully resolved results, but you can also access the raw results + // immediately by examining the additional { queries, results } properties of + // the RefetchQueriesResult object. + Promise> & { + // An array of ObservableQuery objects corresponding 1:1 to TResult values + // in the results arrays (both the TResult[] array below, and the results + // array resolved by the Promise above). + queries: ObservableQuery[]; + // These are the raw TResult values returned by any onQueryUpdated functions + // that were invoked by client.refetchQueries. + results: TResult[]; + }; + // Used by QueryManager["refetchQueries"] export interface InternalRefetchQueriesOptions< TCache extends ApolloCache, diff --git a/src/utilities/index.ts b/src/utilities/index.ts index 7617a9187b3..9657e447ac1 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -86,3 +86,5 @@ export * from './common/arrays'; export * from './common/errorHandling'; export * from './common/canUse'; export * from './common/compact'; + +export * from './types/IsStrictlyAny'; diff --git a/src/utilities/types/IsStrictlyAny.ts b/src/utilities/types/IsStrictlyAny.ts new file mode 100644 index 00000000000..a27932666d4 --- /dev/null +++ b/src/utilities/types/IsStrictlyAny.ts @@ -0,0 +1,17 @@ +// If (and only if) T is any, the union 'a' | 'b' is returned here, representing +// both branches of this conditional type. Only UnionForAny produces this +// union type; all other inputs produce the 'b' literal type. +type UnionForAny = T extends never ? 'a' : 'b'; + +// If that 'A' | 'B' union is then passed to UnionToIntersection, the result +// should be 'A' & 'B', which TypeScript simplifies to the never type, since the +// literal string types 'A' and 'B' are incompatible. More explanation of this +// helper type: https://stackoverflow.com/a/50375286/62076 +type UnionToIntersection = + (U extends any ? (k: U) => void : never) extends + ((k: infer I) => void) ? I : never + +// Returns true if T is any, or false for any other type. +// From https://stackoverflow.com/a/61625296/128454 +export type IsStrictlyAny = + UnionToIntersection> extends never ? true : false; From 30139fc9dd901e376e4205b2d3b61e677408f3fa Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 16:48:48 -0400 Subject: [PATCH 35/47] Improve comments about IsStrictlyAny and related types. --- src/utilities/types/IsStrictlyAny.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/utilities/types/IsStrictlyAny.ts b/src/utilities/types/IsStrictlyAny.ts index a27932666d4..6607dac5945 100644 --- a/src/utilities/types/IsStrictlyAny.ts +++ b/src/utilities/types/IsStrictlyAny.ts @@ -1,17 +1,17 @@ -// If (and only if) T is any, the union 'a' | 'b' is returned here, representing +// Returns true if T is any, or false for any other type. +// Inspired by https://stackoverflow.com/a/61625296/128454. +export type IsStrictlyAny = + UnionToIntersection> extends never ? true : false; + +// If (and only if) T is any, the union 'a' | 1 is returned here, representing // both branches of this conditional type. Only UnionForAny produces this -// union type; all other inputs produce the 'b' literal type. -type UnionForAny = T extends never ? 'a' : 'b'; +// union type; all other inputs produce the 1 literal type. +type UnionForAny = T extends never ? 'a' : 1; -// If that 'A' | 'B' union is then passed to UnionToIntersection, the result -// should be 'A' & 'B', which TypeScript simplifies to the never type, since the -// literal string types 'A' and 'B' are incompatible. More explanation of this -// helper type: https://stackoverflow.com/a/50375286/62076 +// If that 'a' | 1 union is then passed to UnionToIntersection, the result +// should be 'a' & 1, which TypeScript simplifies to the never type, since the +// literal type 'a' and the literal type 1 are incompatible. More explanation of +// this helper type: https://stackoverflow.com/a/50375286/62076. type UnionToIntersection = (U extends any ? (k: U) => void : never) extends ((k: infer I) => void) ? I : never - -// Returns true if T is any, or false for any other type. -// From https://stackoverflow.com/a/61625296/128454 -export type IsStrictlyAny = - UnionToIntersection> extends never ? true : false; From 028fd7510dbb0b04f788204e47eb4fb43148eb8e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 16:53:41 -0400 Subject: [PATCH 36/47] Use interface instead of type for RefetchQueriesResult. --- src/core/types.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/core/types.ts b/src/core/types.ts index 15b9c004905..eaabe06972a 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -73,20 +73,20 @@ export type RefetchQueriesPromiseResults = // to client.refetchQueries. TResult[]; -export type RefetchQueriesResult = - // The result of client.refetchQueries is thenable/awaitable, if you just want - // an array of fully resolved results, but you can also access the raw results - // immediately by examining the additional { queries, results } properties of - // the RefetchQueriesResult object. - Promise> & { - // An array of ObservableQuery objects corresponding 1:1 to TResult values - // in the results arrays (both the TResult[] array below, and the results - // array resolved by the Promise above). - queries: ObservableQuery[]; - // These are the raw TResult values returned by any onQueryUpdated functions - // that were invoked by client.refetchQueries. - results: TResult[]; - }; +// The result of client.refetchQueries is thenable/awaitable, if you just want +// an array of fully resolved results, but you can also access the raw results +// immediately by examining the additional { queries, results } properties of +// the RefetchQueriesResult object. +export interface RefetchQueriesResult +extends Promise> { + // An array of ObservableQuery objects corresponding 1:1 to TResult values + // in the results arrays (both the TResult[] array below, and the results + // array resolved by the Promise above). + queries: ObservableQuery[]; + // These are the raw TResult values returned by any onQueryUpdated functions + // that were invoked by client.refetchQueries. + results: TResult[]; +} // Used by QueryManager["refetchQueries"] export interface InternalRefetchQueriesOptions< From 8ccfbb2b82c83bd2d91d67d61df9a68f085e2db9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 17:26:38 -0400 Subject: [PATCH 37/47] Add a test of returning true from onQueryUpdated. Notice that the type of `results` in client.refetchQueries(...).then(results => ...) is correctly inferred to be ApolloQueryResult[], because the onQueryUpdated function returned true, which triggers a refetch, which returns a Promise>. --- src/__tests__/refetchQueries.ts | 76 ++++++++++++++++++++++++++++++--- 1 file changed, 69 insertions(+), 7 deletions(-) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index 41474f2d409..89fab376a92 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -120,7 +120,9 @@ describe("client.refetchQueries", () => { } else if (obs === abObs) { expect(diff.result).toEqual({ a: "Ayy", b: "B" }); } else { - reject("unexpected ObservableQuery"); + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); } return Promise.resolve(diff.result); }, @@ -152,7 +154,9 @@ describe("client.refetchQueries", () => { } else if (obs === abObs) { expect(diff.result).toEqual({ a: "Ayy", b: "Bee" }); } else { - reject("unexpected ObservableQuery"); + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); } return diff.result; }, @@ -199,7 +203,9 @@ describe("client.refetchQueries", () => { } else if (obs === abObs) { expect(diff.result).toEqual({ a: "Ayy", b: "B" }); } else { - reject("unexpected ObservableQuery"); + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); } return Promise.resolve(diff.result); }, @@ -236,7 +242,9 @@ describe("client.refetchQueries", () => { } else if (obs === abObs) { expect(diff.result).toEqual({ a: "Ayy", b: "Bee" }); } else { - reject("unexpected ObservableQuery"); + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); } return diff.result; }, @@ -352,7 +360,9 @@ describe("client.refetchQueries", () => { } else if (obs === abObs) { expect(diff.result).toEqual({ a: "A", b: "B" }); } else { - reject("unexpected ObservableQuery"); + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); } return diff.result; @@ -377,8 +387,60 @@ describe("client.refetchQueries", () => { resolve(); }); - it("can return true from onQueryUpdated to choose default refetching behavior", () => { - // TODO + itAsync("can return true from onQueryUpdated to choose default refetching behavior", async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const refetchResult = client.refetchQueries({ + include: ["A", "B"], + 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) { + reject("abQuery should not have been updated"); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + return true; + }, + }); + + expect(refetchResult.results.length).toBe(2); + refetchResult.results.forEach(result => { + expect(result).toBeInstanceOf(Promise); + }); + + expect(refetchResult.queries.map(obs => { + expect(obs).toBeInstanceOf(ObservableQuery); + return obs.queryName; + }).sort()).toEqual(["A", "B"]); + + const results = (await refetchResult).map(result => { + // These results are ApolloQueryResult, as inferred by TypeScript. + expect(Object.keys(result).sort()).toEqual([ + "data", + "loading", + "networkStatus", + ]); + return result.data; + }); + + sortObjects(results); + + expect(results).toEqual([ + { a: "A" }, + { b: "B" }, + ]); + + resolve(); }); it("can return false from onQueryUpdated to skip the updated query", () => { From b000064322c3cabc82a0ae0106e8a467ce8c755e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 17:33:05 -0400 Subject: [PATCH 38/47] Add a test of returning false from onQueryUpdated. --- src/__tests__/refetchQueries.ts | 56 +++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index 89fab376a92..d628ad0810c 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -443,8 +443,60 @@ describe("client.refetchQueries", () => { resolve(); }); - it("can return false from onQueryUpdated to skip the updated query", () => { - // TODO + itAsync("can return false from onQueryUpdated to skip the updated query", async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const refetchResult = client.refetchQueries({ + include: ["A", "B"], + 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) { + reject("abQuery should not have been updated"); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + // Skip refetching all but the B query. + return obs.queryName === "B"; + }, + }); + + expect(refetchResult.results.length).toBe(1); + refetchResult.results.forEach(result => { + expect(result).toBeInstanceOf(Promise); + }); + + expect(refetchResult.queries.map(obs => { + expect(obs).toBeInstanceOf(ObservableQuery); + return obs.queryName; + }).sort()).toEqual(["B"]); + + const results = (await refetchResult).map(result => { + // These results are ApolloQueryResult, as inferred by TypeScript. + expect(Object.keys(result).sort()).toEqual([ + "data", + "loading", + "networkStatus", + ]); + return result.data; + }); + + sortObjects(results); + + expect(results).toEqual([ + { b: "B" }, + ]); + + resolve(); }); it("can refetch no-cache queries", () => { From dcb8288e341676498ca594943ff9df0843b6bb52 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 17:34:56 -0400 Subject: [PATCH 39/47] Clarify remaining TODO about testing no-cache with refetchQueries. --- src/__tests__/refetchQueries.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index d628ad0810c..be9330e93b1 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -500,6 +500,7 @@ describe("client.refetchQueries", () => { }); it("can refetch no-cache queries", () => { - // TODO + // TODO The options.updateCache function won't work for these queries, but + // the options.include array should work, at least. }); }); From 0c3fad6f56ab47f6326a01dccb18f18f40d81527 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 19:08:52 -0400 Subject: [PATCH 40/47] Failing test of using updateCache and returning true from onQueryUpdated. --- src/__tests__/refetchQueries.ts | 85 ++++++++++++++++++++++++++++++++- src/cache/core/types/Cache.ts | 2 +- 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index be9330e93b1..597a6aa17e4 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -443,7 +443,90 @@ describe("client.refetchQueries", () => { resolve(); }); - itAsync("can return false from onQueryUpdated to skip the updated query", async (resolve, reject) => { + itAsync("can return true from onQueryUpdated when using options.updateCache", async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const refetchResult = client.refetchQueries({ + updateCache(cache) { + cache.writeQuery({ + query: bQuery, + data: { + b: "Beetlejuice" + }, + }); + }, + + onQueryUpdated(obs, diff) { + if (obs === aObs) { + reject("aQuery should not have been updated"); + } else if (obs === bObs) { + expect(diff.result).toEqual({ b: "Beetlejuice" }); + } else if (obs === abObs) { + expect(diff.result).toEqual({ a: "A", b: "Beetlejuice" }); + } else { + reject(`unexpected ObservableQuery ${ + obs.queryId + } with name ${obs.queryName}`); + } + + expect(client.cache.extract()).toEqual({ + ROOT_QUERY: { + __typename: "Query", + a: "A", + b: "Beetlejuice", + }, + }); + + return true; + }, + }); + + expect(refetchResult.results.length).toBe(2); + refetchResult.results.forEach(result => { + expect(result).toBeInstanceOf(Promise); + }); + + expect(refetchResult.queries.map(obs => { + expect(obs).toBeInstanceOf(ObservableQuery); + return obs.queryName; + }).sort()).toEqual(["AB", "B"]); + + const results = (await refetchResult).map(result => { + // These results are ApolloQueryResult, as inferred by TypeScript. + expect(Object.keys(result).sort()).toEqual([ + "data", + "loading", + "networkStatus", + ]); + return result.data; + }); + + sortObjects(results); + + expect(results).toEqual([ + // Since we returned true from onQueryUpdated, the results were refetched, + // replacing "Beetlejuice" with "B" again. + { a: "A", b: "B"}, + { b: "B" }, + ]); + + expect(client.cache.extract()).toEqual({ + ROOT_QUERY: { + __typename: "Query", + a: "A", + b: "B", + }, + }); + + resolve(); + }); + + itAsync("can return false from onQueryUpdated to skip/ignore a query", async (resolve, reject) => { const client = makeClient(); const [ aObs, diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts index e9ff92021c4..98a21afd4f5 100644 --- a/src/cache/core/types/Cache.ts +++ b/src/cache/core/types/Cache.ts @@ -62,7 +62,7 @@ export namespace Cache { // Passing a string for this option creates a new optimistic layer, with the // given string as its layer.id, just like passing a string for the // optimisticId parameter of performTransaction. Passing true is the same as - // passing undefined to performTransaction (runing the batch operation + // passing undefined to performTransaction (running the batch operation // against the current top layer of the cache), and passing false is the // same as passing null (running the operation against root/non-optimistic // cache data). From c948655ba0919107088f19e761a5b701b70f7fd9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 19:29:07 -0400 Subject: [PATCH 41/47] Inline maybeAddResult helper function. --- src/core/QueryManager.ts | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 9a067905d1c..fdc55bb2a03 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1065,22 +1065,6 @@ export class QueryManager { } const results = new Map, TResult>(); - function maybeAddResult(oq: ObservableQuery, result: any): boolean { - // The onQueryUpdated function can return false to ignore this query and - // skip its normal broadcast, or true to allow the usual broadcast to - // happen (when diff.result has changed). - if (result === false || result === true) { - return result; - } - - // Returning anything other than true or false from onQueryUpdated will - // cause the result to be included in the results Map, while also - // canceling/overriding the normal broadcast. - results.set(oq, result); - - // Prevent the normal cache broadcast of this result. - return false; - } if (updateCache) { this.cache.batch({ @@ -1134,8 +1118,25 @@ export class QueryManager { if (oq) { if (onQueryUpdated) { includedQueriesById.delete(oq.queryId); - return maybeAddResult(oq, onQueryUpdated(oq, diff, lastDiff)); + + const result = onQueryUpdated(oq, diff, lastDiff); + + // The onQueryUpdated function can return false to ignore this + // query and skip its normal broadcast, or true to allow the usual + // broadcast to happen (when diff.result has changed). + if (result === true || result === false) { + return result; + } + + // Returning anything other than true or false from onQueryUpdated + // will cause the result to be included in the results Map, while + // also canceling/overriding the normal broadcast. + results.set(oq, result); + + // Prevent the normal cache broadcast of this result. + return false; } + if (onQueryUpdated !== null) { // If we don't have an onQueryUpdated function, and onQueryUpdated // was not disabled by passing null, make sure this query is @@ -1185,7 +1186,9 @@ export class QueryManager { if (!onQueryUpdated || result === true) { result = fallback(); } - maybeAddResult(oq, result); + if (result !== false) { + results.set(oq, result as TResult); + } } }); } From 01b13c70b7092c50fd402a23bfd98f29302012c7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 19:35:16 -0400 Subject: [PATCH 42/47] Store both lastDiff and diff in includedQueriesById. --- src/core/QueryManager.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index fdc55bb2a03..e4becc82e7a 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1048,7 +1048,8 @@ export class QueryManager { ): Map, TResult> { const includedQueriesById = new Map | undefined; + lastDiff?: Cache.DiffResult; + diff?: Cache.DiffResult; }>(); if (include) { @@ -1056,7 +1057,7 @@ export class QueryManager { getQueryIdsForQueryDescriptor(this, desc).forEach(queryId => { includedQueriesById.set(queryId, { desc, - diff: typeof desc === "string" + lastDiff: typeof desc === "string" ? this.getQuery(queryId).getDiff() : void 0, }); @@ -1143,6 +1144,7 @@ export class QueryManager { // "included" like any other options.include-specified query. includedQueriesById.set(oq.queryId, { desc: oq.queryName || ``, + lastDiff, diff, }); } @@ -1152,7 +1154,7 @@ export class QueryManager { } if (includedQueriesById.size) { - includedQueriesById.forEach(({ desc, diff }, queryId) => { + includedQueriesById.forEach(({ desc, lastDiff, diff }, queryId) => { const queryInfo = this.getQuery(queryId); let oq = queryInfo.observableQuery; let fallback: undefined | (() => any); @@ -1180,8 +1182,11 @@ export class QueryManager { // queries, even the PureQueryOptions ones. Otherwise, we call the // fallback function defined above. if (onQueryUpdated) { - queryInfo.reset(); // Force queryInfo.getDiff() to read from cache. - result = onQueryUpdated(oq, queryInfo.getDiff(), diff); + 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(); From 75e455c7b2b83fbac4752eb8b7874e15680e2d0a Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 20:03:44 -0400 Subject: [PATCH 43/47] Fix failing test of updateCache with onQueryUpdated returning true. --- src/core/QueryManager.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index e4becc82e7a..153b10d6ec2 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -1118,23 +1118,27 @@ export class QueryManager { if (oq) { if (onQueryUpdated) { + // 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); - const result = onQueryUpdated(oq, diff, lastDiff); + let result = onQueryUpdated(oq, diff, lastDiff); - // The onQueryUpdated function can return false to ignore this - // query and skip its normal broadcast, or true to allow the usual - // broadcast to happen (when diff.result has changed). - if (result === true || result === false) { - return result; + if (result === true) { + // The onQueryUpdated function requested the default refetching + // behavior by returning true. + result = oq.refetch() as any; } - // Returning anything other than true or false from onQueryUpdated - // will cause the result to be included in the results Map, while - // also canceling/overriding the normal broadcast. - results.set(oq, result); + // Record the result in the results Map, as long as onQueryUpdated + // did not return false to skip/ignore this result. + if (result !== false) { + results.set(oq, result as TResult); + } - // Prevent the normal cache broadcast of this result. + // Prevent the normal cache broadcast of this result, since we've + // already handled it. return false; } From 311f6927429072d78a68fac42d40ab1290a1e332 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 20:18:34 -0400 Subject: [PATCH 44/47] Use fewer forced typecasts in QueryManager#refetchQueries. --- src/core/ApolloClient.ts | 13 +++++++++---- src/core/QueryManager.ts | 19 +++++++++++-------- src/core/types.ts | 9 ++++++++- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index a2ab7b299c3..53485777b07 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -17,6 +17,7 @@ import { Resolvers, RefetchQueriesOptions, RefetchQueriesResult, + InternalRefetchQueriesResult, } from './types'; import { @@ -544,18 +545,22 @@ export class ApolloClient implements DataProxy { ): RefetchQueriesResult { const map = this.queryManager.refetchQueries(options); const queries: ObservableQuery[] = []; - const results: TResult[] = []; + const results: InternalRefetchQueriesResult[] = []; - map.forEach((update, obsQuery) => { + map.forEach((result, obsQuery) => { queries.push(obsQuery); - results.push(update); + results.push(result); }); - const result = Promise.all(results) as RefetchQueriesResult; + const result = Promise.all( + results as TResult[] + ) as RefetchQueriesResult; + // In case you need the raw results immediately, without awaiting // Promise.all(results): result.queries = queries; result.results = results; + return result; } diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 153b10d6ec2..8c8880b1282 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -39,6 +39,8 @@ import { RefetchQueryDescription, InternalRefetchQueriesOptions, RefetchQueryDescriptor, + InternalRefetchQueriesResult, + InternalRefetchQueriesMap, } from './types'; import { LocalState } from './LocalState'; @@ -1045,7 +1047,7 @@ export class QueryManager { removeOptimistic = optimistic ? makeUniqueId("refetchQueries") : void 0, onQueryUpdated, }: InternalRefetchQueriesOptions, TResult> - ): Map, TResult> { + ): InternalRefetchQueriesMap { const includedQueriesById = new Map; @@ -1065,7 +1067,7 @@ export class QueryManager { }); } - const results = new Map, TResult>(); + const results: InternalRefetchQueriesMap = new Map; if (updateCache) { this.cache.batch({ @@ -1123,18 +1125,19 @@ export class QueryManager { // options.include. includedQueriesById.delete(oq.queryId); - let result = onQueryUpdated(oq, diff, lastDiff); + let result: boolean | InternalRefetchQueriesResult = + onQueryUpdated(oq, diff, lastDiff); if (result === true) { // The onQueryUpdated function requested the default refetching // behavior by returning true. - result = oq.refetch() as any; + result = oq.refetch(); } // Record the result in the results Map, as long as onQueryUpdated // did not return false to skip/ignore this result. if (result !== false) { - results.set(oq, result as TResult); + results.set(oq, result); } // Prevent the normal cache broadcast of this result, since we've @@ -1161,7 +1164,7 @@ export class QueryManager { includedQueriesById.forEach(({ desc, lastDiff, diff }, queryId) => { const queryInfo = this.getQuery(queryId); let oq = queryInfo.observableQuery; - let fallback: undefined | (() => any); + let fallback: undefined | (() => Promise>); if (typeof desc === "string") { fallback = () => oq!.refetch(); @@ -1181,7 +1184,7 @@ export class QueryManager { } if (oq && fallback) { - let result: boolean | TResult | undefined; + 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. @@ -1196,7 +1199,7 @@ export class QueryManager { result = fallback(); } if (result !== false) { - results.set(oq, result as TResult); + results.set(oq, result!); } } }); diff --git a/src/core/types.ts b/src/core/types.ts index eaabe06972a..f3c4644385e 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -85,7 +85,7 @@ extends Promise> { queries: ObservableQuery[]; // These are the raw TResult values returned by any onQueryUpdated functions // that were invoked by client.refetchQueries. - results: TResult[]; + results: InternalRefetchQueriesResult[]; } // Used by QueryManager["refetchQueries"] @@ -101,6 +101,13 @@ export interface InternalRefetchQueriesOptions< removeOptimistic?: string; } +export type InternalRefetchQueriesResult = + TResult | Promise>; + +export type InternalRefetchQueriesMap = + Map, + InternalRefetchQueriesResult>; + export type OperationVariables = Record; export type PureQueryOptions = { From c2bce76690b3f43a5c21b59679c7e99d11467622 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 13 May 2021 20:38:31 -0400 Subject: [PATCH 45/47] Move makeUniqueId from QueryManager.ts into @apollo/client/utilities. --- src/__tests__/__snapshots__/exports.ts.snap | 1 + src/core/QueryManager.ts | 8 +------- src/utilities/common/makeUniqueId.ts | 9 +++++++++ src/utilities/index.ts | 1 + 4 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 src/utilities/common/makeUniqueId.ts diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index cf064e3d32a..b292f4d5144 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -353,6 +353,7 @@ Array [ "isReference", "iterateObserversSafely", "makeReference", + "makeUniqueId", "maybeDeepFreeze", "mergeDeep", "mergeDeepArray", diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 8c8880b1282..24a3b64d0dc 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -19,6 +19,7 @@ import { isNonEmptyArray, Concast, ConcastSourcesIterable, + makeUniqueId, } from '../utilities'; import { ApolloError, isApolloError } from '../errors'; import { @@ -1414,10 +1415,3 @@ function getQueryIdsForQueryDescriptor( } return queryIds; } - -const prefixCounts: Record = Object.create(null); -function makeUniqueId(prefix: string) { - const count = prefixCounts[prefix] || 1; - prefixCounts[prefix] = count + 1; - return `${prefix}:${count}:${Math.random().toString(36).slice(2)}`; -} diff --git a/src/utilities/common/makeUniqueId.ts b/src/utilities/common/makeUniqueId.ts new file mode 100644 index 00000000000..b0e804bd7fb --- /dev/null +++ b/src/utilities/common/makeUniqueId.ts @@ -0,0 +1,9 @@ +const prefixCounts = new Map(); + +// These IDs won't be globally unique, but they will be unique within this +// process, thanks to the counter, and unguessable thanks to the random suffix. +export function makeUniqueId(prefix: string) { + const count = prefixCounts.get(prefix) || 1; + prefixCounts.set(prefix, count + 1); + return `${prefix}:${count}:${Math.random().toString(36).slice(2)}`; +} diff --git a/src/utilities/index.ts b/src/utilities/index.ts index 9657e447ac1..91540d00e2d 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -86,5 +86,6 @@ export * from './common/arrays'; export * from './common/errorHandling'; export * from './common/canUse'; export * from './common/compact'; +export * from './common/makeUniqueId'; export * from './types/IsStrictlyAny'; From 512804cdc6919d0ce04139f18443ede92b81d7c6 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 18 May 2021 16:25:48 -0400 Subject: [PATCH 46/47] Allow including client.refetchQueries queries by DocumentNode. --- src/__tests__/__snapshots__/exports.ts.snap | 1 + src/__tests__/refetchQueries.ts | 89 +++++++++++++++++++++ src/core/QueryManager.ts | 12 +-- src/core/types.ts | 4 +- src/utilities/graphql/storeUtils.ts | 10 +++ src/utilities/index.ts | 1 + 6 files changed, 110 insertions(+), 7 deletions(-) diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index b292f4d5144..3032b34917e 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -347,6 +347,7 @@ Array [ "graphQLResultHasError", "hasClientExports", "hasDirectives", + "isDocumentNode", "isField", "isInlineFragment", "isNonEmptyArray", diff --git a/src/__tests__/refetchQueries.ts b/src/__tests__/refetchQueries.ts index 597a6aa17e4..d5b5bd72236 100644 --- a/src/__tests__/refetchQueries.ts +++ b/src/__tests__/refetchQueries.ts @@ -262,6 +262,95 @@ describe("client.refetchQueries", () => { resolve(); }); + itAsync("includes query DocumentNode objects specified in options.include", async (resolve, reject) => { + const client = makeClient(); + const [ + aObs, + bObs, + abObs, + ] = await setup(client); + + const ayyResults = await client.refetchQueries({ + updateCache(cache) { + cache.writeQuery({ + query: aQuery, + data: { + a: "Ayy", + }, + }); + }, + + // Note that we're passing query DocumentNode objects instead of query + // name strings, in this test. + include: [bQuery, abQuery], + + 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" }, + // Included this time! + { b: "B" }, + ]); + + const beeResults = await client.refetchQueries({ + updateCache(cache) { + cache.writeQuery({ + query: bQuery, + data: { + b: "Bee", + }, + }); + }, + + // The abQuery and "AB" should be redundant, but the aQuery here is + // important for aObs to be included. + include: [abQuery, "AB", aQuery], + + 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" }, // Included this time! + { a: "Ayy", b: "Bee" }, + { b: "Bee" }, + ]); + + unsubscribe(); + resolve(); + }); + itAsync("refetches watched queries if onQueryUpdated not provided", async (resolve, reject) => { const client = makeClient(); const [ diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 24a3b64d0dc..021d19fccf9 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -20,6 +20,7 @@ import { Concast, ConcastSourcesIterable, makeUniqueId, + isDocumentNode, } from '../utilities'; import { ApolloError, isApolloError } from '../errors'; import { @@ -1060,7 +1061,7 @@ export class QueryManager { getQueryIdsForQueryDescriptor(this, desc).forEach(queryId => { includedQueriesById.set(queryId, { desc, - lastDiff: typeof desc === "string" + lastDiff: typeof desc === "string" || isDocumentNode(desc) ? this.getQuery(queryId).getDiff() : void 0, }); @@ -1167,7 +1168,7 @@ export class QueryManager { let oq = queryInfo.observableQuery; let fallback: undefined | (() => Promise>); - if (typeof desc === "string") { + if (typeof desc === "string" || isDocumentNode(desc)) { fallback = () => oq!.refetch(); } else if (desc && typeof desc === "object") { const options = { @@ -1395,10 +1396,11 @@ function getQueryIdsForQueryDescriptor( desc: RefetchQueryDescriptor, ) { const queryIds: string[] = []; - if (typeof desc === "string") { - qm["queries"].forEach(({ observableQuery: oq }, queryId) => { + const isName = typeof desc === "string"; + if (isName || isDocumentNode(desc)) { + qm["queries"].forEach(({ observableQuery: oq, document }, queryId) => { if (oq && - oq.queryName === desc && + desc === (isName ? oq.queryName : document) && oq.hasObservers()) { queryIds.push(queryId); } diff --git a/src/core/types.ts b/src/core/types.ts index f3c4644385e..a6907179a34 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -22,7 +22,7 @@ export type OnQueryUpdated = ( lastDiff: Cache.DiffResult | undefined, ) => boolean | TResult; -export type RefetchQueryDescriptor = string | PureQueryOptions; +export type RefetchQueryDescriptor = string | DocumentNode | PureQueryOptions; export type RefetchQueryDescription = RefetchQueryDescriptor[]; // Used by ApolloClient["refetchQueries"] @@ -36,7 +36,7 @@ export interface RefetchQueriesOptions< // 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?: string[]; + include?: Exclude[]; optimistic?: boolean; // If no onQueryUpdated function is provided, any queries affected by the // updateCache function or included in the options.include array will be diff --git a/src/utilities/graphql/storeUtils.ts b/src/utilities/graphql/storeUtils.ts index 70e0bc0eb5b..a0aa5e8d2d1 100644 --- a/src/utilities/graphql/storeUtils.ts +++ b/src/utilities/graphql/storeUtils.ts @@ -15,6 +15,7 @@ import { SelectionNode, NameNode, SelectionSetNode, + DocumentNode, } from 'graphql'; import { InvariantError } from 'ts-invariant'; @@ -48,6 +49,15 @@ export interface StoreObject { [storeFieldName: string]: StoreValue; } +export function isDocumentNode(value: any): value is DocumentNode { + return ( + value !== null && + typeof value === "object" && + (value as DocumentNode).kind === "Document" && + Array.isArray((value as DocumentNode).definitions) + ); +} + function isStringValue(value: ValueNode): value is StringValueNode { return value.kind === 'StringValue'; } diff --git a/src/utilities/index.ts b/src/utilities/index.ts index 91540d00e2d..c878b83622b 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -33,6 +33,7 @@ export { Directives, VariableValue, makeReference, + isDocumentNode, isReference, isField, isInlineFragment, From 491eeafcccc88d7043d1b0eb29357595996e3a4b Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 18 May 2021 16:44:04 -0400 Subject: [PATCH 47/47] Update CHANGELOG.md to reflect changes made in PR #8000. --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d5abd61956..6f4e29418fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,12 +34,15 @@ - `InMemoryCache` supports a new method called `batch`, which is similar to `performTransaction` but takes named options rather than positional parameters. One of these named options is an `onDirty(watch, diff)` callback, which can be used to determine which watched queries were invalidated by the `batch` operation.
[@benjamn](https://github.com/benjamn) in [#7819](https://github.com/apollographql/apollo-client/pull/7819) -- Mutations now accept an optional callback function called `reobserveQuery`, which will be passed the `ObservableQuery` and `Cache.DiffResult` objects for any queries invalidated by cache writes performed by the mutation's final `update` function. Using `reobserveQuery`, you can override the default `FetchPolicy` of the query, by (for example) calling `ObservableQuery` methods like `refetch` to force a network request. This automatic detection of invalidated queries provides an alternative to manually enumerating queries using the `refetchQueries` mutation option. Also, if you return a `Promise` from `reobserveQuery`, the mutation will automatically await that `Promise`, rendering the `awaitRefetchQueries` option unnecessary.
+- Mutations now accept an optional callback function called `onQueryUpdated`, which will be passed the `ObservableQuery` and `Cache.DiffResult` objects for any queries invalidated by cache writes performed by the mutation's final `update` function. Using `onQueryUpdated`, you can override the default `FetchPolicy` of the query, by (for example) calling `ObservableQuery` methods like `refetch` to force a network request. This automatic detection of invalidated queries provides an alternative to manually enumerating queries using the `refetchQueries` mutation option. Also, if you return a `Promise` from `onQueryUpdated`, the mutation will automatically await that `Promise`, rendering the `awaitRefetchQueries` option unnecessary.
[@benjamn](https://github.com/benjamn) in [#7827](https://github.com/apollographql/apollo-client/pull/7827) - Support `client.refetchQueries` as an imperative way to refetch queries, without having to pass `options.refetchQueries` to `client.mutate`.
[@dannycochran](https://github.com/dannycochran) in [#7431](https://github.com/apollographql/apollo-client/pull/7431) +- Improve standalone `client.refetchQueries` method to support automatic detection of queries needing to be refetched.
+ [@benjamn](https://github.com/benjamn) in [#8000](https://github.com/apollographql/apollo-client/pull/8000) + - When `@apollo/client` is imported as CommonJS (for example, in Node.js), the global `process` variable is now shadowed with a stripped-down object that includes only `process.env.NODE_ENV` (since that's all Apollo Client needs), eliminating the significant performance penalty of repeatedly accessing `process.env` at runtime.
[@benjamn](https://github.com/benjamn) in [#7627](https://github.com/apollographql/apollo-client/pull/7627)