diff --git a/.changeset/tough-ghosts-itch.md b/.changeset/tough-ghosts-itch.md new file mode 100644 index 00000000000..59267cab40b --- /dev/null +++ b/.changeset/tough-ghosts-itch.md @@ -0,0 +1,7 @@ +--- +"@apollo/client": patch +--- + +Fix: unblocks support for defer in mutations + +If the `@defer` directive is present in the document passed to `mutate`, the Promise will resolve with the final merged data after the last multipart chunk has arrived in the response. diff --git a/config/bundlesize.ts b/config/bundlesize.ts index b628fb88013..38df3b2f8e4 100644 --- a/config/bundlesize.ts +++ b/config/bundlesize.ts @@ -3,7 +3,7 @@ import { join } from "path"; import { gzipSync } from "zlib"; import bytes from "bytes"; -const gzipBundleByteLengthLimit = bytes("31.87KB"); +const gzipBundleByteLengthLimit = bytes("32KB"); const minFile = join("dist", "apollo-client.min.cjs"); const minPath = join(__dirname, "..", minFile); const gzipByteLen = gzipSync(readFileSync(minPath)).byteLength; diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index b16e03c1870..f8dfc3175d0 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -369,6 +369,7 @@ Array [ "getFragmentDefinitions", "getFragmentFromSelection", "getFragmentQueryDocument", + "getGraphQLErrorsFromResult", "getInclusionDirectives", "getMainDefinition", "getOperationDefinition", diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts index d5aa94ad878..30edbc55cf1 100644 --- a/src/core/QueryInfo.ts +++ b/src/core/QueryInfo.ts @@ -3,6 +3,7 @@ import { equal } from "@wry/equality"; import { Cache, ApolloCache } from '../cache'; import { DeepMerger } from "../utilities" +import { mergeIncrementalData } from '../utilities/common/incrementalResult'; import { WatchQueryOptions, ErrorPolicy } from './watchQueryOptions'; import { ObservableQuery, reobserveCacheFirst } from './ObservableQuery'; import { QueryListener } from './types'; @@ -373,21 +374,7 @@ export class QueryInfo { this.reset(); if ('incremental' in result && isNonEmptyArray(result.incremental)) { - let mergedData = this.getDiff().result; - - result.incremental.forEach(({ data, path, errors }) => { - for (let i = path.length - 1; i >= 0; --i) { - const key = path[i]; - const isNumericKey = !isNaN(+key); - const parent: Record = isNumericKey ? [] : {}; - parent[key] = data; - data = parent as typeof data; - } - if (errors) { - graphQLErrors.push(...errors); - } - mergedData = merger.merge(mergedData, data); - }); + const mergedData = mergeIncrementalData(this.getDiff().result, result); result.data = mergedData; // Detect the first chunk of a deferred query and merge it with existing diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 36fac27ef13..ecebf55c554 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -6,7 +6,10 @@ type OperationTypeNode = any; import { equal } from '@wry/equality'; import { ApolloLink, execute, FetchResult } from '../link/core'; -import { isExecutionPatchIncrementalResult } from '../utilities/common/incrementalResult'; +import { + isExecutionPatchIncrementalResult, + isExecutionPatchResult, +} from '../utilities/common/incrementalResult'; import { Cache, ApolloCache, canonicalStringify } from '../cache'; import { @@ -15,6 +18,7 @@ import { getOperationName, hasClientExports, graphQLResultHasError, + getGraphQLErrorsFromResult, removeConnectionDirectiveFromDocument, canUseWeakMap, ObservableSubscription, @@ -27,6 +31,7 @@ import { isDocumentNode, isNonNullObject, } from '../utilities'; +import { mergeIncrementalData } from '../utilities/common/incrementalResult'; import { ApolloError, isApolloError } from '../errors'; import { QueryOptions, @@ -248,7 +253,7 @@ export class QueryManager { (result: FetchResult) => { if (graphQLResultHasError(result) && errorPolicy === 'none') { throw new ApolloError({ - graphQLErrors: result.errors, + graphQLErrors: getGraphQLErrorsFromResult(result), }); } @@ -295,13 +300,14 @@ export class QueryManager { next(storeResult) { 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); + // Since mutations might receive multiple payloads from the + // ApolloLink chain (e.g. when used with @defer), + // we resolve with a SingleExecutionResult or after the final + // ExecutionPatchResult has arrived and we have assembled the + // multipart response into a single result. + if (!('hasNext' in storeResult) || storeResult.hasNext === false) { + resolve(storeResult); + } }, error(err: Error) { @@ -355,12 +361,38 @@ export class QueryManager { const skipCache = mutation.fetchPolicy === "no-cache"; if (!skipCache && shouldWriteResult(result, mutation.errorPolicy)) { - cacheWrites.push({ - result: result.data, - dataId: 'ROOT_MUTATION', - query: mutation.document, - variables: mutation.variables, - }); + if (!isExecutionPatchIncrementalResult(result)) { + cacheWrites.push({ + result: result.data, + dataId: 'ROOT_MUTATION', + query: mutation.document, + variables: mutation.variables, + }); + } + if (isExecutionPatchIncrementalResult(result) && isNonEmptyArray(result.incremental)) { + const diff = cache.diff({ + id: "ROOT_MUTATION", + // The cache complains if passed a mutation where it expects a + // query, so we transform mutations and subscriptions to queries + // (only once, thanks to this.transformCache). + query: this.transform(mutation.document).asQuery, + variables: mutation.variables, + optimistic: false, + returnPartialData: true, + }); + const mergedData = mergeIncrementalData(diff.result, result); + if (typeof mergedData !== 'undefined') { + // cast the ExecutionPatchResult to FetchResult here since + // ExecutionPatchResult never has `data` when returned from the server + (result as FetchResult).data = mergedData; + cacheWrites.push({ + result: mergedData, + dataId: 'ROOT_MUTATION', + query: mutation.document, + variables: mutation.variables, + }) + } + } const { updateQueries } = mutation; if (updateQueries) { @@ -421,6 +453,12 @@ export class QueryManager { // apply those writes to the store by running this reducer again with // a write action. const { update } = mutation; + // Determine whether result is a SingleExecutionResult, + // or the final ExecutionPatchResult. + const isFinalResult = + !isExecutionPatchResult(result) || + (isExecutionPatchIncrementalResult(result) && !result.hasNext); + if (update) { if (!skipCache) { // Re-read the ROOT_MUTATION data we just wrote into the cache @@ -438,20 +476,31 @@ export class QueryManager { returnPartialData: true, }); - if (diff.complete && !(isExecutionPatchIncrementalResult(result))) { - result = { ...result, data: diff.result }; + if (diff.complete) { + result = { ...result as FetchResult, data: diff.result }; + if ('incremental' in result) { + delete result.incremental; + } + if ('hasNext' in result) { + delete result.hasNext; + } } } - update(cache, result, { - context: mutation.context, - variables: mutation.variables, - }); + // If we've received the whole response, + // either a SingleExecutionResult or the final ExecutionPatchResult, + // call the update function. + if (isFinalResult) { + update(cache, result, { + context: mutation.context, + variables: mutation.variables, + }); + } } // TODO Do this with cache.evict({ id: 'ROOT_MUTATION' }) but make it // shallow to allow rolling back optimistic evictions. - if (!skipCache && !mutation.keepRootFields) { + if (!skipCache && !mutation.keepRootFields && isFinalResult) { cache.modify({ id: 'ROOT_MUTATION', fields(value, { fieldName, DELETE }) { @@ -1053,19 +1102,8 @@ export class QueryManager { ), result => { - const graphQLErrors = isNonEmptyArray(result.errors) - ? result.errors.slice(0) - : []; - - if ('incremental' in result && isNonEmptyArray(result.incremental)) { - result.incremental.forEach(incrementalResult => { - if (incrementalResult.errors) { - graphQLErrors.push(...incrementalResult.errors); - } - }); - } - - const hasErrors = isNonEmptyArray(graphQLErrors); + const graphQLErrors = getGraphQLErrorsFromResult(result); + const hasErrors = graphQLErrors.length > 0; // If we interrupted this request by calling getResultsFromLink again // with the same QueryInfo object, we ignore the old results. diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index 4a0acd2b205..3d0eaf0f3d0 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -9,11 +9,12 @@ import fetchMock from "fetch-mock"; import { ApolloClient, ApolloLink, ApolloQueryResult, Cache, NetworkStatus, Observable, ObservableQuery, TypedDocumentNode } from '../../../core'; import { InMemoryCache } from '../../../cache'; -import { itAsync, MockedProvider, mockSingleLink, subscribeAndCount } from '../../../testing'; +import { itAsync, MockedProvider, MockSubscriptionLink, mockSingleLink, subscribeAndCount } from '../../../testing'; import { ApolloProvider } from '../../context'; import { useQuery } from '../useQuery'; import { useMutation } from '../useMutation'; import { BatchHttpLink } from '../../../link/batch-http'; +import { FetchResult } from '../../../link/core'; describe('useMutation Hook', () => { interface Todo { @@ -2206,4 +2207,248 @@ describe('useMutation Hook', () => { await screen.findByText('item 3'); }); }); + describe('defer', () => { + const CREATE_TODO_MUTATION_DEFER = gql` + mutation createTodo($description: String!, $priority: String) { + createTodo(description: $description, priority: $priority) { + id + ... @defer { + description + priority + } + } + } + `; + const variables = { + description: 'Get milk!' + }; + it('resolves a deferred mutation with the full result', async () => { + const errorSpy = jest.spyOn(console, "error"); + const link = new MockSubscriptionLink(); + + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const useCreateTodo = () => { + const [createTodo, { loading, data }] = useMutation( + CREATE_TODO_MUTATION_DEFER + ); + + useEffect(() => { + createTodo({ variables }); + }, [variables]); + + return { loading, data }; + }; + + const { result, waitForNextUpdate } = renderHook( + () => useCreateTodo(), + { + wrapper: ({ children }) => ( + + {children} + + ), + }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + + setTimeout(() => { + link.simulateResult({ + result: { + data: { + createTodo: { + id: 1, + __typename: 'Todo', + }, + }, + hasNext: true + }, + }); + }); + + setTimeout(() => { + link.simulateResult({ + result: { + incremental: [{ + data: { + description: 'Get milk!', + priority: 'High', + __typename: 'Todo', + }, + path: ['createTodo'], + }], + hasNext: false + }, + }, true); + }); + + + // When defer is used in a mutation, the final value resolves + // in a single result + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toEqual({ + createTodo: { + id: 1, + description: "Get milk!", + priority: "High", + __typename: 'Todo', + }, + }); + expect(errorSpy).not.toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + it('resolves with resulting errors and calls onError callback', async () => { + const errorSpy = jest.spyOn(console, "error"); + const link = new MockSubscriptionLink(); + + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const onError = jest.fn(); + const { result } = renderHook( + () => useMutation(CREATE_TODO_MUTATION_DEFER, { onError }), + { + wrapper: ({ children }) => ( + + {children} + + ), + }, + ); + + const createTodo = result.current[0]; + + let fetchResult: any; + + setTimeout(() => { + link.simulateResult({ + result: { + data: { + createTodo: { + id: 1, + __typename: 'Todo', + }, + }, + hasNext: true + }, + }); + }); + + setTimeout(() => { + link.simulateResult({ + result: { + incremental: [{ + data: null, + errors: [ + new GraphQLError(CREATE_TODO_ERROR) + ], + path: ['createTodo'], + }], + hasNext: false + }, + }, true); + }); + await act(async () => { + fetchResult = await createTodo({ variables }); + }); + + expect(fetchResult.errors.message).toBe(CREATE_TODO_ERROR); + expect(fetchResult.data).toBe(undefined); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError.mock.calls[0][0].message).toBe(CREATE_TODO_ERROR); + expect(errorSpy).not.toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + it('calls the update function with the final merged result data', async () => { + const errorSpy = jest.spyOn(console, "error"); + const link = new MockSubscriptionLink(); + const update = jest.fn(); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const { result } = renderHook( + () => useMutation(CREATE_TODO_MUTATION_DEFER, + { update }), + { + wrapper: ({ children }) => ( + + {children} + + ), + }, + ); + const [createTodo] = result.current; + + let promiseReturnedByMutate: Promise; + + await act(async () => { + promiseReturnedByMutate = createTodo({ variables }); + }); + + link.simulateResult({ + result: { + data: { + createTodo: { + id: 1, + __typename: 'Todo', + }, + }, + hasNext: true + }, + }); + + link.simulateResult({ + result: { + incremental: [{ + data: { + description: 'Get milk!', + priority: 'High', + __typename: 'Todo', + }, + path: ['createTodo'], + }], + hasNext: false + }, + }, true); + + await act(async () => { + await promiseReturnedByMutate; + }); + + expect(update).toHaveBeenCalledTimes(1); + expect(update).toHaveBeenCalledWith( + // the first item is the cache, which we don't need to make any + // assertions against in this test + expect.anything(), + // second argument is the result + expect.objectContaining({ + data: { + createTodo: { + id: 1, + description: "Get milk!", + priority: "High", + __typename: 'Todo', + }, + } + }), + // third argument is an object containing context and variables + // but we only care about variables here + expect.objectContaining({ variables }) + ); + + expect(errorSpy).not.toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + }); }); diff --git a/src/utilities/common/errorHandling.ts b/src/utilities/common/errorHandling.ts index 5d60f57695f..ba622d663e4 100644 --- a/src/utilities/common/errorHandling.ts +++ b/src/utilities/common/errorHandling.ts @@ -1,5 +1,26 @@ -import { ExecutionResult } from 'graphql'; +import { FetchResult } from "../../link/core"; +import { isNonEmptyArray } from "../../utilities/common/arrays"; +import { isExecutionPatchIncrementalResult } from "../../utilities/common/incrementalResult"; -export function graphQLResultHasError(result: ExecutionResult): boolean { - return (result.errors && result.errors.length > 0) || false; +export function graphQLResultHasError(result: FetchResult): boolean { + const errors = getGraphQLErrorsFromResult(result); + return isNonEmptyArray(errors); +} + +export function getGraphQLErrorsFromResult(result: FetchResult) { + const graphQLErrors = isNonEmptyArray(result.errors) + ? result.errors.slice(0) + : []; + + if ( + isExecutionPatchIncrementalResult(result) && + isNonEmptyArray(result.incremental) + ) { + result.incremental.forEach((incrementalResult) => { + if (incrementalResult.errors) { + graphQLErrors.push(...incrementalResult.errors); + } + }); + } + return graphQLErrors; } diff --git a/src/utilities/common/incrementalResult.ts b/src/utilities/common/incrementalResult.ts index ddb6be317cc..68f4f2f1469 100644 --- a/src/utilities/common/incrementalResult.ts +++ b/src/utilities/common/incrementalResult.ts @@ -1,5 +1,53 @@ -import { ExecutionPatchIncrementalResult } from '../../link/core'; +import { + ExecutionPatchIncrementalResult, + ExecutionPatchInitialResult, + ExecutionPatchResult, + FetchResult, +} from "../../link/core"; +import { isNonEmptyArray } from "./arrays"; +import { DeepMerger } from "./mergeDeep"; -export function isExecutionPatchIncrementalResult(value: any): value is ExecutionPatchIncrementalResult { - return !!(value as ExecutionPatchIncrementalResult).incremental; +export function isExecutionPatchIncrementalResult( + value: FetchResult +): value is ExecutionPatchIncrementalResult { + return "incremental" in value; +} + +export function isExecutionPatchInitialResult( + value: FetchResult +): value is ExecutionPatchInitialResult { + return "hasNext" in value && "data" in value; +} + +export function isExecutionPatchResult( + value: FetchResult +): value is ExecutionPatchResult { + return ( + isExecutionPatchIncrementalResult(value) || + isExecutionPatchInitialResult(value) + ); +} + +export function mergeIncrementalData( + prevResult: TData, + result: ExecutionPatchResult +) { + let mergedData = prevResult; + const merger = new DeepMerger(); + if ( + isExecutionPatchIncrementalResult(result) && + isNonEmptyArray(result.incremental) + ) { + result.incremental.forEach(({ data, path }) => { + for (let i = path.length - 1; i >= 0; --i) { + const key = path[i]; + const isNumericKey = !isNaN(+key); + const parent: Record = isNumericKey ? [] : {}; + parent[key] = data; + data = parent as typeof data; + } + mergedData = merger.merge(mergedData, data); + }); + } + return mergedData as TData; }