fix nextFetchPolicy behaviour with transformed documents (#11235)

apollographql · Sep 28, 2023 · 6cddaaf · 6cddaaf
1 parent 76539d9
commit 6cddaaf
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 40 deletions.
diff --git a/.changeset/early-snakes-cheer.md b/.changeset/early-snakes-cheer.md
@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix nextFetchPolicy behaviour with transformed documents by keeping `options` reference stable when passing it through QueryManager.
diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts
@@ -728,15 +728,17 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
 
   private fetch(
     options: WatchQueryOptions<TVariables, TData>,
-    newNetworkStatus?: NetworkStatus
+    newNetworkStatus?: NetworkStatus,
+    query?: DocumentNode
   ) {
     // TODO Make sure we update the networkStatus (and infer fetchVariables)
     // before actually committing to the fetch.
     this.queryManager.setObservableQuery(this);
     return this.queryManager["fetchConcastWithInfo"](
       this.queryId,
       options,
-      newNetworkStatus
+      newNetworkStatus,
+      query
     );
   }
 
@@ -883,20 +885,15 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
       }
     }
 
-    // If the transform doesn't change the document, leave `options` alone and
-    // use the original object.
-    const fetchOptions =
-      query === options.query ? options : { ...options, query };
-
-    this.waitForOwnResult &&= skipCacheDataFor(fetchOptions.fetchPolicy);
+    this.waitForOwnResult &&= skipCacheDataFor(options.fetchPolicy);
     const finishWaitingForOwnResult = () => {
       if (this.concast === concast) {
         this.waitForOwnResult = false;
       }
     };
 
-    const variables = fetchOptions.variables && { ...fetchOptions.variables };
-    const { concast, fromLink } = this.fetch(fetchOptions, newNetworkStatus);
+    const variables = options.variables && { ...options.variables };
+    const { concast, fromLink } = this.fetch(options, newNetworkStatus, query);
     const observer: Observer<ApolloQueryResult<TData>> = {
       next: (result) => {
         finishWaitingForOwnResult();

diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
@@ -1220,9 +1220,9 @@ export class QueryManager<TStore> {
     // The initial networkStatus for this fetch, most often
     // NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
     // or setVariables.
-    networkStatus = NetworkStatus.loading
+    networkStatus = NetworkStatus.loading,
+    query = options.query
   ): ConcastAndInfo<TData> {
-    const { query } = options;
     const variables = this.getVariables(query, options.variables) as TVars;
     const queryInfo = this.getQuery(queryId);
 

diff --git a/src/core/__tests__/fetchPolicies.ts b/src/core/__tests__/fetchPolicies.ts
@@ -814,16 +814,24 @@ describe("cache-and-network", function () {
 
 describe("nextFetchPolicy", () => {
   type TData = {
-    linkCounter: number;
-    opName: string;
-    opVars: Record<string, any>;
+    echo: {
+      linkCounter: number;
+      opName: string;
+      opVars: Record<string, any>;
+    };
+  };
+
+  type TVars = {
+    refetching?: boolean;
   };
 
   const EchoQuery: TypedDocumentNode<TData> = gql`
     query EchoQuery {
-      linkCounter
-      opName
-      opVars
+      echo {
+        linkCounter
+        opName
+        opVars
+      }
     }
   `;
 
@@ -835,9 +843,12 @@ describe("nextFetchPolicy", () => {
           setTimeout(() => {
             observer.next({
               data: {
-                linkCounter: ++linkCounter,
-                opName: request.operationName,
-                opVars: request.variables,
+                echo: {
+                  __typename: "Echo",
+                  linkCounter: ++linkCounter,
+                  opName: request.operationName,
+                  opVars: request.variables,
+                },
               },
             });
             observer.complete();
@@ -846,9 +857,9 @@ describe("nextFetchPolicy", () => {
     );
   }
 
-  const checkNextFetchPolicy = <TData, TVars extends object>(args: {
+  const checkNextFetchPolicy = (args: {
     fetchPolicy: WatchQueryFetchPolicy;
-    nextFetchPolicy: WatchQueryOptions<TVars, TData>["nextFetchPolicy"];
+    nextFetchPolicy: WatchQueryOptions<{}, TData>["nextFetchPolicy"];
     useDefaultOptions: boolean;
     onResult(info: {
       count: number;
@@ -867,7 +878,9 @@ describe("nextFetchPolicy", () => {
       (resolve, reject) => {
         const client = new ApolloClient({
           link: makeLink(),
-          cache: new InMemoryCache(),
+          cache: new InMemoryCache({
+            addTypename: true,
+          }),
           defaultOptions: {
             watchQuery: args.useDefaultOptions
               ? {
@@ -917,7 +930,8 @@ describe("nextFetchPolicy", () => {
   }) => {
     if (count === 1) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 1,
         opName: "EchoQuery",
         opVars: {},
@@ -930,7 +944,8 @@ describe("nextFetchPolicy", () => {
           refetching: true,
         })
         .then((result) => {
-          expect(result.data).toEqual({
+          expect(result.data.echo).toEqual({
+            __typename: "Echo",
             linkCounter: 2,
             opName: "EchoQuery",
             opVars: {
@@ -941,7 +956,8 @@ describe("nextFetchPolicy", () => {
         .catch(reject);
     } else if (count === 2) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 2,
         opName: "EchoQuery",
         opVars: {
@@ -959,7 +975,8 @@ describe("nextFetchPolicy", () => {
         })
         .then((result) => {
           expect(result.loading).toBe(false);
-          expect(result.data).toEqual({
+          expect(result.data.echo).toEqual({
+            __typename: "Echo",
             linkCounter: 3,
             opName: "EchoQuery",
             opVars: {
@@ -973,7 +990,8 @@ describe("nextFetchPolicy", () => {
       expect(observable.options.fetchPolicy).toBe("cache-first");
     } else if (count === 3) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 3,
         opName: "EchoQuery",
         opVars: {
@@ -1044,7 +1062,8 @@ describe("nextFetchPolicy", () => {
   }) => {
     if (count === 1) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 1,
         opName: "EchoQuery",
         opVars: {},
@@ -1057,7 +1076,8 @@ describe("nextFetchPolicy", () => {
           refetching: true,
         })
         .then((result) => {
-          expect(result.data).toEqual({
+          expect(result.data.echo).toEqual({
+            __typename: "Echo",
             linkCounter: 2,
             opName: "EchoQuery",
             opVars: {
@@ -1068,7 +1088,8 @@ describe("nextFetchPolicy", () => {
         .catch(reject);
     } else if (count === 2) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 2,
         opName: "EchoQuery",
         opVars: {
@@ -1086,7 +1107,8 @@ describe("nextFetchPolicy", () => {
         })
         .then((result) => {
           expect(result.loading).toBe(false);
-          expect(result.data).toEqual({
+          expect(result.data.echo).toEqual({
+            __typename: "Echo",
             linkCounter: 3,
             opName: "EchoQuery",
             opVars: {
@@ -1100,7 +1122,8 @@ describe("nextFetchPolicy", () => {
       // expect(observable.options.fetchPolicy).toBe("cache-and-network");
     } else if (count === 3) {
       expect(result.loading).toBe(true);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 2,
         opName: "EchoQuery",
         opVars: {
@@ -1112,7 +1135,8 @@ describe("nextFetchPolicy", () => {
       expect(observable.options.fetchPolicy).toBe("cache-first");
     } else if (count === 4) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 3,
         opName: "EchoQuery",
         opVars: {
@@ -1191,7 +1215,8 @@ describe("nextFetchPolicy", () => {
   }) => {
     if (count === 1) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 1,
         opName: "EchoQuery",
         opVars: {},
@@ -1204,7 +1229,8 @@ describe("nextFetchPolicy", () => {
           refetching: true,
         })
         .then((result) => {
-          expect(result.data).toEqual({
+          expect(result.data.echo).toEqual({
+            __typename: "Echo",
             linkCounter: 2,
             opName: "EchoQuery",
             opVars: {
@@ -1215,7 +1241,8 @@ describe("nextFetchPolicy", () => {
         .catch(reject);
     } else if (count === 2) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 2,
         opName: "EchoQuery",
         opVars: {
@@ -1233,7 +1260,8 @@ describe("nextFetchPolicy", () => {
         })
         .then((result) => {
           expect(result.loading).toBe(false);
-          expect(result.data).toEqual({
+          expect(result.data.echo).toEqual({
+            __typename: "Echo",
             linkCounter: 2,
             opName: "EchoQuery",
             opVars: {
@@ -1250,7 +1278,8 @@ describe("nextFetchPolicy", () => {
       expect(observable.options.fetchPolicy).toBe("cache-first");
     } else if (count === 3) {
       expect(result.loading).toBe(false);
-      expect(result.data).toEqual({
+      expect(result.data.echo).toEqual({
+        __typename: "Echo",
         linkCounter: 2,
         opName: "EchoQuery",
         opVars: {