Repository: apollographql/apollo-client

Problem statement:
🐛: Mutations with @defer only responds with the first part
> To clarify, this is happening from a non-federated AS4 instance, not router.

**Intended outcome:**
Using the `useMutation` hook with a mutation with `@defer` results in only the first chunk loading. 

For example, given a server responding with: 

```

---
content-type: application/json; charset=utf-8

{"hasNext":true,"data":{"makePayment":{"id":"1afa84cc-26a5-4820-a496-33c8366e03cd","__typename":"MakePaymentResult"}}}
---
content-type: application/json; charset=utf-8

{"hasNext":false,"incremental":[{"path":["makePayment"],"data":{"paymentStatus":{"__typename":"PaymentSuccess","id":"2a3a1ddd-81e1-4535-8a9c-e8dc1ce38026","billedAmount":39.854330085558395},"__typename":"MakePaymentResult"}}]}
-----
```

Only 
```
{"makePayment":{"id":"1afa84cc-26a5-4820-a496-33c8366e03cd","__typename":"MakePaymentResult"}
``` 
is reported in the `data` object. 

**Actual outcome:**
Mutations update with the correct data, same as with `useQuery` which works as expected. 

**How to reproduce the issue:**
Example Repo: https://github.com/lleadbet/graphql-galaxy-demo

**Versions**
```
  System:
    OS: macOS 12.6
  Binaries:
    Node: 16.16.0 - ~/.volta/tools/image/node/16.16.0/bin/node
    Yarn: 1.22.19 - ~/.volta/tools/image/yarn/1.22.19/bin/yarn
    npm: 8.11.0 - ~/.volta/tools/image/node/16.16.0/bin/npm
  Browsers:
    Chrome: 107.0.5304.110
    Firefox: 106.0.5
    Safari: 16.0
  npmPackages:
    @apollo/client: ^3.7.1 => 3.7.1 
   ```


Hints:
This will require some changes to `QueryManager` most likely, inside of the `markMutationResult` method.
@alessbell  just to confirm, is there any change Apollo Router has to consider here or is this strictly on the client side? cc @abernix 
@jpvajda I don't have a test case that works against the router yet, but I'm not sure if the issue is in my local subgraph. @abernix, is there a mutation example you can share? If not I will go ahead and open a bug.
To clarify, this is happening from a non-federated AS4 instance, not router
Yep - I was referring to my attempt to reproduce the issue using the router. When I use a subgraph with the same schema used in https://github.com/lleadbet/graphql-galaxy-demo, I get a "cannot query field 'makePayment' on type 'Query'" error from the router.
@lleadbet That's good info, thanks for clarifying that.
Opened the router issue here: https://github.com/apollographql/router/issues/2099

Patch:
diff --git a/config/bundlesize.ts b/config/bundlesize.ts
--- 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/core/QueryInfo.ts b/src/core/QueryInfo.ts
--- 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<string | number, any> = 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
--- 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<TStore> {
         (result: FetchResult<TData>) => {
           if (graphQLResultHasError(result) && errorPolicy === 'none') {
             throw new ApolloError({
-              graphQLErrors: result.errors,
+              graphQLErrors: getGraphQLErrorsFromResult(result),
             });
           }
 
@@ -295,13 +300,14 @@ export class QueryManager<TStore> {
         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<TStore> {
     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<TData>({
+          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<TStore> {
           // 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<TStore> {
                 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<TStore> {
       ),
 
       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/utilities/common/errorHandling.ts b/src/utilities/common/errorHandling.ts
--- 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<unknown>): 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
--- 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<TData>(
+  prevResult: TData,
+  result: ExecutionPatchResult<TData>
+) {
+  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<string | number, any> = isNumericKey ? [] : {};
+        parent[key] = data;
+        data = parent as typeof data;
+      }
+      mergedData = merger.merge(mergedData, data);
+    });
+  }
+  return mergedData as TData;
 }


Test patch:
diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap
--- 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/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx
--- 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 }) => (
+            <ApolloProvider client={client}>
+              {children}
+            </ApolloProvider>
+          ),
+        },
+      );
+
+      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 }) => (
+            <ApolloProvider client={client}>
+              {children}
+            </ApolloProvider>
+          ),
+        },
+      );
+
+      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 }) => (
+            <ApolloProvider client={client}>
+              {children}
+            </ApolloProvider>
+          ),
+        },
+      );
+      const [createTodo] = result.current;
+
+      let promiseReturnedByMutate: Promise<FetchResult>;
+
+      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();
+    });
+  });
 });