Ignore data values set in subsequent chunks in incremental responses (#12982)

Author: jerelmiller

.api-reports/api-report-incremental.api.md

@@ -52,8 +52,6 @@ namespace Defer20220824Handler {
     };
     // (undocumented)
     type SubsequentResult<TData = Record<string, unknown>> = {
-        data?: TData | null | undefined;
-        errors?: ReadonlyArray<GraphQLFormattedError>;
         extensions?: Record<string, unknown>;
         hasNext: boolean;
         incremental?: Array<IncrementalResult<TData>>;

.changeset/neat-windows-compete.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Ignore top-level `data` values on subsequent chunks in incremental responses.

.changeset/shaggy-brooms-talk.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix the `Defer20220824Handler.SubsequentResult` type to match the `FormattedSubsequentIncrementalExecutionResult` type in `[email protected]`.

.size-limits.json

@@ -1,6 +1,6 @@
 {
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 44831,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 39452,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 44773,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 39426,
   "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 33875,
   "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27756
 }

src/incremental/handlers/__tests__/defer20220824/defer.test.ts

@@ -1037,3 +1037,78 @@ test("handles final chunk of { hasNext: false } correctly in usage with Apollo C
   });
   await expect(observableStream).not.toEmitAnything();
 });
+
+// Servers that return a `data` property in subsequent payloads are technically
+// invalid, but we still want to handle cases where the server misbehaves.
+//
+// See the following issue for more information:
+// https://github.com/apollographql/apollo-client/issues/12976
+test("ignores `data` property added to subsequent chunks by misbehaving servers", async () => {
+  const { httpLink, enqueueInitialChunk, enqueueSubsequentChunk } =
+    mockDefer20220824();
+  const client = new ApolloClient({
+    link: httpLink,
+    cache: new InMemoryCache(),
+    incrementalHandler: new Defer20220824Handler(),
+  });
+
+  const query = gql`
+    query HeroNameQuery {
+      hero {
+        id
+        ... @defer {
+          name
+        }
+      }
+    }
+  `;
+
+  const observableStream = new ObservableStream(client.watchQuery({ query }));
+
+  enqueueInitialChunk({
+    data: { hero: { __typename: "Hero", id: "1" } },
+    hasNext: true,
+  });
+
+  await expect(observableStream).toEmitTypedValue({
+    loading: true,
+    data: undefined,
+    dataState: "empty",
+    networkStatus: NetworkStatus.loading,
+    partial: true,
+  });
+
+  await expect(observableStream).toEmitTypedValue({
+    loading: true,
+    data: markAsStreaming({
+      hero: {
+        __typename: "Hero",
+        id: "1",
+      },
+    }),
+    dataState: "streaming",
+    networkStatus: NetworkStatus.streaming,
+    partial: true,
+  });
+
+  enqueueSubsequentChunk({
+    // @ts-expect-error simulate misbehaving server
+    data: null,
+    incremental: [{ data: { name: "Luke" }, path: ["hero"] }],
+    hasNext: false,
+  });
+
+  await expect(observableStream).toEmitTypedValue({
+    loading: false,
+    data: {
+      hero: {
+        __typename: "Hero",
+        id: "1",
+        name: "Luke",
+      },
+    },
+    dataState: "complete",
+    networkStatus: NetworkStatus.ready,
+    partial: false,
+  });
+});

src/incremental/handlers/__tests__/graphql17Alpha9/defer.test.ts

@@ -2629,3 +2629,79 @@ test("handles final chunk of { hasNext: false } correctly in usage with Apollo C
   });
   await expect(observableStream).not.toEmitAnything();
 });
+
+// Servers that return a `data` property in subsequent payloads are technically
+// invalid, but we still want to handle cases where the server misbehaves.
+//
+// See the following issue for more information:
+// https://github.com/apollographql/apollo-client/issues/12976
+test("ignores `data` property added to subsequent chunks by misbehaving servers", async () => {
+  const stream = mockDeferStreamGraphQL17Alpha9();
+  const client = new ApolloClient({
+    link: stream.httpLink,
+    cache: new InMemoryCache(),
+    incrementalHandler: new GraphQL17Alpha9Handler(),
+  });
+
+  const query = gql`
+    query HeroNameQuery {
+      hero {
+        id
+        ... @defer {
+          name
+        }
+      }
+    }
+  `;
+
+  const observableStream = new ObservableStream(client.watchQuery({ query }));
+
+  stream.enqueueInitialChunk({
+    data: { hero: { __typename: "Hero", id: "1" } },
+    pending: [{ id: "0", path: ["hero"] }],
+    hasNext: true,
+  });
+
+  await expect(observableStream).toEmitTypedValue({
+    loading: true,
+    data: undefined,
+    dataState: "empty",
+    networkStatus: NetworkStatus.loading,
+    partial: true,
+  });
+
+  await expect(observableStream).toEmitTypedValue({
+    loading: true,
+    data: markAsStreaming({
+      hero: {
+        __typename: "Hero",
+        id: "1",
+      },
+    }),
+    dataState: "streaming",
+    networkStatus: NetworkStatus.streaming,
+    partial: true,
+  });
+
+  stream.enqueueSubsequentChunk({
+    // @ts-expect-error simulate misbehaving server
+    data: null,
+    incremental: [{ data: { name: "Luke" }, id: "0" }],
+    completed: [{ id: "0" }],
+    hasNext: false,
+  });
+
+  await expect(observableStream).toEmitTypedValue({
+    loading: false,
+    data: {
+      hero: {
+        __typename: "Hero",
+        id: "1",
+        name: "Luke",
+      },
+    },
+    dataState: "complete",
+    networkStatus: NetworkStatus.ready,
+    partial: false,
+  });
+});

src/incremental/handlers/defer20220824.ts

@@ -33,8 +33,6 @@ export declare namespace Defer20220824Handler {
   };
 
   export type SubsequentResult<TData = Record<string, unknown>> = {
-    data?: TData | null | undefined;
-    errors?: ReadonlyArray<GraphQLFormattedError>;
     extensions?: Record<string, unknown>;
     hasNext: boolean;
     incremental?: Array<IncrementalResult<TData>>;
@@ -99,7 +97,6 @@ class DeferRequest<TData extends Record<string, unknown>>
   ): FormattedExecutionResult<TData> {
     this.hasNext = chunk.hasNext;
     this.data = cacheData;
-    this.merge(chunk);
 
     if (hasIncrementalChunks(chunk)) {
       for (const incremental of chunk.incremental) {
@@ -135,6 +132,8 @@ class DeferRequest<TData extends Record<string, unknown>>
           arrayMerge
         );
       }
+    } else {
+      this.merge(chunk);
     }
 
     const result: FormattedExecutionResult<TData> = { data: this.data };
@@ -178,7 +177,9 @@ export class Defer20220824Handler
       }
     };
     if (this.isIncrementalResult(result)) {
-      push(result);
+      if ("errors" in result) {
+        push(result);
+      }
       if (hasIncrementalChunks(result)) {
         result.incremental.forEach(push);
       }

src/incremental/handlers/graphql17Alpha9.ts

@@ -119,8 +119,6 @@ class IncrementalRequest<TData>
       }
     }
 
-    this.merge(chunk, "truncate");
-
     if (hasIncrementalChunks(chunk)) {
       for (const incremental of chunk.incremental) {
         const pending = this.pending.find(({ id }) => incremental.id === id);
@@ -194,6 +192,8 @@ class IncrementalRequest<TData>
           arrayMerge
         );
       }
+    } else {
+      this.merge(chunk, "truncate");
     }
 
     if ("completed" in chunk && chunk.completed) {
@@ -286,7 +286,7 @@ export class GraphQL17Alpha9Handler
 
     if (this.isIncrementalResult(result)) {
       push(new IncrementalRequest().handle(undefined, result));
-    } else {
+    } else if ("errors" in result) {
       push(result);
     }
 

src/link/context/__tests__/index.ts

@@ -1,3 +1,5 @@
+import assert from "node:assert";
+
 import { gql } from "graphql-tag";
 import { Observable, of } from "rxjs";
 
@@ -268,6 +270,8 @@ test("can access the client from operation argument", async () => {
   const link = withContext.concat(mockLink);
   const stream = new ObservableStream(execute(link, { query }, { client }));
 
-  const { data } = await stream.takeNext();
-  expect(data!.client).toBe(client);
+  const result = await stream.takeNext();
+
+  assert("data" in result);
+  expect(result.data!.client).toBe(client);
 });