From 331a17e3ef3aee9b0e100c1974bb935b922a6460 Mon Sep 17 00:00:00 2001 From: Jan Zimmek Date: Sun, 18 Mar 2018 14:37:21 +0100 Subject: [PATCH 1/7] await refetched queries before resolve mutation --- .../apollo-client/src/core/QueryManager.ts | 68 ++++++++++--------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/packages/apollo-client/src/core/QueryManager.ts b/packages/apollo-client/src/core/QueryManager.ts index 5c668ea5de9..0add95edcde 100644 --- a/packages/apollo-client/src/core/QueryManager.ts +++ b/packages/apollo-client/src/core/QueryManager.ts @@ -229,45 +229,49 @@ export class QueryManager { }), ); }, - complete: () => { - if (error) { - this.mutationStore.markMutationError(mutationId, error); - } - - this.dataStore.markMutationComplete({ - mutationId, - optimisticResponse, - }); - - this.broadcastQueries(); + complete: async () => { + try { + if (error) { + this.mutationStore.markMutationError(mutationId, error); + } - if (error) { - reject(error); - return; - } + this.dataStore.markMutationComplete({ + mutationId, + optimisticResponse, + }); - // allow for conditional refetches - // XXX do we want to make this the only API one day? - if (typeof refetchQueries === 'function') - refetchQueries = refetchQueries(storeResult as ExecutionResult); + this.broadcastQueries(); - refetchQueries.forEach(refetchQuery => { - if (typeof refetchQuery === 'string') { - this.refetchQueryByName(refetchQuery); + if (error) { + reject(error); return; } - this.query({ - query: refetchQuery.query, - variables: refetchQuery.variables, - fetchPolicy: 'network-only', - }); - }); - this.setQuery(mutationId, () => ({ document: undefined })); - if (errorPolicy === 'ignore' && storeResult && storeResult.errors) { - delete storeResult.errors; + // allow for conditional refetches + // XXX do we want to make this the only API one day? + if (typeof refetchQueries === 'function') + refetchQueries = refetchQueries(storeResult as ExecutionResult); + + for (const refetchQuery of refetchQueries) { + if (typeof refetchQuery === 'string') { + await this.refetchQueryByName(refetchQuery); + continue; + } + + await this.query({ + query: refetchQuery.query, + variables: refetchQuery.variables, + fetchPolicy: 'network-only', + }); + } + this.setQuery(mutationId, () => ({ document: undefined })); + if (errorPolicy === 'ignore' && storeResult && storeResult.errors) { + delete storeResult.errors; + } + resolve(storeResult as FetchResult); + } catch (completionError) { + reject(completionError); } - resolve(storeResult as FetchResult); }, }); }); From 06d39749d7a00e0efeef91f18a563d823bcc883b Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 23 Jul 2018 13:41:07 -0400 Subject: [PATCH 2/7] Adjust refetch promises to run in parallel; Remove try-catch --- .../apollo-client/src/core/QueryManager.ts | 102 ++++++++++-------- 1 file changed, 57 insertions(+), 45 deletions(-) diff --git a/packages/apollo-client/src/core/QueryManager.ts b/packages/apollo-client/src/core/QueryManager.ts index ed59a206148..d5420216a52 100644 --- a/packages/apollo-client/src/core/QueryManager.ts +++ b/packages/apollo-client/src/core/QueryManager.ts @@ -192,6 +192,61 @@ export class QueryManager { ...context, optimisticResponse, }); + + const completeMutation = async () => { + if (error) { + this.mutationStore.markMutationError(mutationId, error); + } + + this.dataStore.markMutationComplete({ + mutationId, + optimisticResponse, + }); + + this.broadcastQueries(); + + // allow for conditional refetches + // XXX do we want to make this the only API one day? + if (typeof refetchQueries === 'function') { + refetchQueries = refetchQueries(storeResult as ExecutionResult); + } + + const refetchQueryPromises: Promise< + ApolloQueryResult[] | ApolloQueryResult<{}> + >[] = []; + + for (const refetchQuery of refetchQueries) { + if (typeof refetchQuery === 'string') { + const promise = this.refetchQueryByName(refetchQuery); + if (promise) { + refetchQueryPromises.push(promise); + } + continue; + } + + refetchQueryPromises.push( + this.query({ + query: refetchQuery.query, + variables: refetchQuery.variables, + fetchPolicy: 'network-only', + }), + ); + } + + await Promise.all(refetchQueryPromises); + + this.setQuery(mutationId, () => ({ document: undefined })); + if ( + errorPolicy === 'ignore' && + storeResult && + graphQLResultHasError(storeResult) + ) { + delete storeResult.errors; + } + + return storeResult as FetchResult; + }; + execute(this.link, operation).subscribe({ next: (result: ExecutionResult) => { if (graphQLResultHasError(result) && errorPolicy === 'none') { @@ -215,6 +270,7 @@ export class QueryManager { } storeResult = result as FetchResult; }, + error: (err: Error) => { this.mutationStore.markMutationError(mutationId, err); this.dataStore.markMutationComplete({ @@ -230,52 +286,8 @@ export class QueryManager { }), ); }, - complete: async () => { - try { - if (error) { - this.mutationStore.markMutationError(mutationId, error); - } - - this.dataStore.markMutationComplete({ - mutationId, - optimisticResponse, - }); - - this.broadcastQueries(); - - // allow for conditional refetches - // XXX do we want to make this the only API one day? - if (typeof refetchQueries === 'function') { - refetchQueries = refetchQueries(storeResult as ExecutionResult); - } - - for (const refetchQuery of refetchQueries) { - if (typeof refetchQuery === 'string') { - await this.refetchQueryByName(refetchQuery); - continue; - } - - await this.query({ - query: refetchQuery.query, - variables: refetchQuery.variables, - fetchPolicy: 'network-only', - }); - } - - this.setQuery(mutationId, () => ({ document: undefined })); - if ( - errorPolicy === 'ignore' && - storeResult && - graphQLResultHasError(storeResult) - ) { - delete storeResult.errors; - } - resolve(storeResult as FetchResult); - } catch (completionError) { - reject(completionError); - } - }, + complete: () => completeMutation().then(resolve, reject), }); }); } From 3a72e7ccdb111c789c95165166f791918bf75920 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 23 Jul 2018 14:16:45 -0400 Subject: [PATCH 3/7] Add `awaitRefetchQueries` option --- packages/apollo-client/src/core/QueryManager.ts | 5 ++++- packages/apollo-client/src/core/watchQueryOptions.ts | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/apollo-client/src/core/QueryManager.ts b/packages/apollo-client/src/core/QueryManager.ts index d5420216a52..db6df9cb44e 100644 --- a/packages/apollo-client/src/core/QueryManager.ts +++ b/packages/apollo-client/src/core/QueryManager.ts @@ -122,6 +122,7 @@ export class QueryManager { optimisticResponse, updateQueries: updateQueriesByName, refetchQueries = [], + awaitRefetchQueries = false, update: updateWithProxyFn, errorPolicy = 'none', fetchPolicy, @@ -233,7 +234,9 @@ export class QueryManager { ); } - await Promise.all(refetchQueryPromises); + if (awaitRefetchQueries) { + await Promise.all(refetchQueryPromises); + } this.setQuery(mutationId, () => ({ document: undefined })); if ( diff --git a/packages/apollo-client/src/core/watchQueryOptions.ts b/packages/apollo-client/src/core/watchQueryOptions.ts index fdbffb7d0fc..addf271d707 100644 --- a/packages/apollo-client/src/core/watchQueryOptions.ts +++ b/packages/apollo-client/src/core/watchQueryOptions.ts @@ -180,6 +180,16 @@ export interface MutationBaseOptions< | ((result: ExecutionResult) => RefetchQueryDescription) | RefetchQueryDescription; + /** + * By default, `refetchQueries` does not wait for the refetched queries to + * be completed, before resolving the mutation `Promise`. This ensures that + * query refetching does not hold up mutation response handling (query + * refetching is handled asynchronously). Set `awaitRefetchQueries` to + * `true` if you would like to wait for the refetched queries to complete, + * before the mutation can be marked as resolved. + */ + awaitRefetchQueries?: boolean; + /** * A function which provides a {@link DataProxy} and the result of the * mutation to allow the user to update the store based on the results of the From 611b9fad73912c4496df6f204acd0478791f8b7b Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 23 Jul 2018 15:01:26 -0400 Subject: [PATCH 4/7] Fix existing functionality tests --- packages/apollo-client/src/core/QueryManager.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/apollo-client/src/core/QueryManager.ts b/packages/apollo-client/src/core/QueryManager.ts index db6df9cb44e..38428200832 100644 --- a/packages/apollo-client/src/core/QueryManager.ts +++ b/packages/apollo-client/src/core/QueryManager.ts @@ -206,6 +206,10 @@ export class QueryManager { this.broadcastQueries(); + if (error) { + throw error; + } + // allow for conditional refetches // XXX do we want to make this the only API one day? if (typeof refetchQueries === 'function') { From aead676dec72bf132fefc6ad1bbb6af4f19923a3 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 23 Jul 2018 15:03:02 -0400 Subject: [PATCH 5/7] Adjust `apollo-client` and `apollo-boost` bundle size max --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index dde6d4d11dd..3ef71a6fade 100644 --- a/package.json +++ b/package.json @@ -34,12 +34,12 @@ { "name": "apollo-client", "path": "./packages/apollo-client/lib/bundle.min.js", - "maxSize": "11.8 kB" + "maxSize": "12.5 kB" }, { "name": "apollo-boost", "path": "./packages/apollo-boost/lib/bundle.min.js", - "maxSize": "35 kB" + "maxSize": "36 kB" }, { "name": "apollo-utilities", From 4315a1a283503fb92a9c8e8c89c8395c63d6240d Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 23 Jul 2018 15:06:07 -0400 Subject: [PATCH 6/7] Changelog updates --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cfa7839e1f..d39a1243404 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ [@ivank](https://github.com/ivank) in [#3598](https://github.com/apollographql/apollo-client/pull/3598) - Add optional generic type params for variables on low level methods.
[@mvestergaard](https://github.com/mvestergaard) in [#3588](https://github.com/apollographql/apollo-client/pull/3588) +- Add a new `awaitRefetchQueries` config option to the Apollo Client + `mutate` function, that when set to `true` will wait for all + `refetchQueries` to be fully refetched, before resolving the mutation + call. `awaitRefetchQueries` is `false` by default.
+ [@jzimmek](https://github.com/jzimmek) in [#](https://github.com/apollographql/apollo-client/pull/3169) ### Apollo Boost (vNext) From 5e9a6ce862f86bc01381d6cb8c22f9fcf3522e0d Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 23 Jul 2018 15:35:56 -0400 Subject: [PATCH 7/7] New `awaitRefetchQueries` tests --- .../src/core/__tests__/QueryManager/index.ts | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/packages/apollo-client/src/core/__tests__/QueryManager/index.ts b/packages/apollo-client/src/core/__tests__/QueryManager/index.ts index 91ad49bdd87..1d8638e67be 100644 --- a/packages/apollo-client/src/core/__tests__/QueryManager/index.ts +++ b/packages/apollo-client/src/core/__tests__/QueryManager/index.ts @@ -3933,6 +3933,7 @@ describe('QueryManager', () => { }); }); }); + describe('refetchQueries', () => { const oldWarn = console.warn; let warned: any; @@ -4447,6 +4448,119 @@ describe('QueryManager', () => { done(); }); }); + + describe('awaitRefetchQueries', () => { + function awaitRefetchTest({ awaitRefetchQueries }) { + const query = gql` + query getAuthors($id: ID!) { + author(id: $id) { + firstName + lastName + } + } + `; + + const queryData = { + author: { + firstName: 'John', + lastName: 'Smith', + }, + }; + + const mutation = gql` + mutation changeAuthorName { + changeAuthorName(newName: "Jack Smith") { + firstName + lastName + } + } + `; + + const mutationData = { + changeAuthorName: { + firstName: 'Jack', + lastName: 'Smith', + }, + }; + + const secondReqData = { + author: { + firstName: 'Jane', + lastName: 'Johnson', + }, + }; + + const variables = { id: '1234' }; + + const queryManager = mockQueryManager( + { + request: { query, variables }, + result: { data: queryData }, + }, + { + request: { query: mutation }, + result: { data: mutationData }, + }, + { + request: { query, variables }, + result: { data: secondReqData }, + }, + ); + + const observable = queryManager.watchQuery({ + query, + variables, + notifyOnNetworkStatusChange: false, + }); + + let mutationComplete = false; + return observableToPromise( + { observable }, + result => { + expect(stripSymbols(result.data)).toEqual(queryData); + const mutateOptions = { + mutation, + refetchQueries: ['getAuthors'], + }; + if (awaitRefetchQueries) { + mutateOptions.awaitRefetchQueries = awaitRefetchQueries; + } + queryManager.mutate(mutateOptions).then(() => { + mutationComplete = true; + }); + }, + result => { + if (awaitRefetchQueries) { + expect(mutationComplete).not.toBeTruthy(); + } else { + expect(mutationComplete).toBeTruthy(); + } + expect(stripSymbols(observable.currentResult().data)).toEqual( + secondReqData, + ); + expect(stripSymbols(result.data)).toEqual(secondReqData); + }, + ); + } + + it( + 'should not wait for `refetchQueries` to complete before resolving ' + + 'the mutation, when `awaitRefetchQueries` is falsy', + () => { + awaitRefetchTest({ awaitRefetchQueries: undefined }); + awaitRefetchTest({ awaitRefetchQueries: false }); + }, + ); + + it( + 'should wait for `refetchQueries` to complete before resolving ' + + 'the mutation, when `awaitRefetchQueries` is `true`', + () => { + awaitRefetchTest({ awaitRefetchQueries: true }); + }, + ); + }); + describe('store watchers', () => { it('does not fill up the store on resolved queries', () => { const query1 = gql`