Fix issue where an empty array returned from fetchMore would rerend…

…er with an empty list (#11706)
apollographql · Mar 20, 2024 · 8619bc7 · 8619bc7
1 parent 591c2b6
commit 8619bc7
Show file tree

Hide file tree

Showing 7 changed files with 252 additions and 15 deletions.
diff --git a/.changeset/curly-berries-hammer.md b/.changeset/curly-berries-hammer.md
@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix issue in all suspense hooks where returning an empty array after calling `fetchMore` would rerender the component with an empty list.
diff --git a/.size-limits.json b/.size-limits.json
@@ -1,4 +1,4 @@
 {
-  "dist/apollo-client.min.cjs": 39267,
+  "dist/apollo-client.min.cjs": 39273,
   "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32630
 }
diff --git a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx
@@ -4795,6 +4795,15 @@ describe("refetch", () => {
 describe("fetchMore", () => {
   it("re-suspends when calling `fetchMore` with different variables", async () => {
     const { query, link } = setupPaginatedCase();
+    const cache = new InMemoryCache({
+      typePolicies: {
+        Query: {
+          fields: {
+            letters: { keyArgs: false },
+          },
+        },
+      },
+    });
     const user = userEvent.setup();
     const Profiler = createDefaultProfiler<PaginatedCaseData>();
     const { SuspenseFallback, ReadQueryHook } =
@@ -4818,7 +4827,7 @@ describe("fetchMore", () => {
       );
     }
 
-    renderWithMocks(<App />, { link, wrapper: Profiler });
+    renderWithMocks(<App />, { cache, link, wrapper: Profiler });
 
     {
       const { renderedComponents } = await Profiler.takeRender();

diff --git a/src/react/hooks/__tests__/useLoadableQuery.test.tsx b/src/react/hooks/__tests__/useLoadableQuery.test.tsx
@@ -49,6 +49,7 @@ import {
   Profiler,
   SimpleCaseData,
   createProfiler,
+  setupPaginatedCase,
   setupSimpleCase,
   spyOnConsole,
   useTrackRenders,
@@ -3205,7 +3206,22 @@ it("`refetch` works with startTransition to allow React to show stale UI until f
 });
 
 it("re-suspends when calling `fetchMore` with different variables", async () => {
-  const { query, client } = usePaginatedQueryCase();
+  const { query, link } = setupPaginatedCase();
+
+  const client = new ApolloClient({
+    link,
+    cache: new InMemoryCache({
+      typePolicies: {
+        Query: {
+          fields: {
+            letters: {
+              keyArgs: false,
+            },
+          },
+        },
+      },
+    }),
+  });
 
   const Profiler = createDefaultProfiler<PaginatedQueryData>();
   const { SuspenseFallback, ReadQueryHook } =
@@ -3244,8 +3260,8 @@ it("re-suspends when calling `fetchMore` with different variables", async () =>
     expect(snapshot.result).toEqual({
       data: {
         letters: [
-          { letter: "A", position: 1 },
-          { letter: "B", position: 2 },
+          { __typename: "Letter", letter: "A", position: 1 },
+          { __typename: "Letter", letter: "B", position: 2 },
         ],
       },
       error: undefined,
@@ -3268,8 +3284,8 @@ it("re-suspends when calling `fetchMore` with different variables", async () =>
     expect(snapshot.result).toEqual({
       data: {
         letters: [
-          { letter: "C", position: 3 },
-          { letter: "D", position: 4 },
+          { __typename: "Letter", letter: "C", position: 3 },
+          { __typename: "Letter", letter: "D", position: 4 },
         ],
       },
       error: undefined,

diff --git a/src/react/hooks/__tests__/useQueryRefHandlers.test.tsx b/src/react/hooks/__tests__/useQueryRefHandlers.test.tsx
@@ -1100,7 +1100,18 @@ test("resuspends when calling `fetchMore`", async () => {
 
   const user = userEvent.setup();
 
-  const client = new ApolloClient({ cache: new InMemoryCache(), link });
+  const client = new ApolloClient({
+    cache: new InMemoryCache({
+      typePolicies: {
+        Query: {
+          fields: {
+            letters: { keyArgs: false },
+          },
+        },
+      },
+    }),
+    link,
+  });
   const preloadQuery = createQueryPreloader(client);
 
   const Profiler = createProfiler({
@@ -1397,7 +1408,18 @@ test("paginates from queryRefs produced by useBackgroundQuery", async () => {
   const { query, link } = setupPaginatedCase();
 
   const user = userEvent.setup();
-  const client = new ApolloClient({ cache: new InMemoryCache(), link });
+  const client = new ApolloClient({
+    cache: new InMemoryCache({
+      typePolicies: {
+        Query: {
+          fields: {
+            letters: { keyArgs: false },
+          },
+        },
+      },
+    }),
+    link,
+  });
 
   const Profiler = createProfiler({
     initialSnapshot: {
@@ -1489,7 +1511,18 @@ test("paginates from queryRefs produced by useLoadableQuery", async () => {
   const { query, link } = setupPaginatedCase();
 
   const user = userEvent.setup();
-  const client = new ApolloClient({ cache: new InMemoryCache(), link });
+  const client = new ApolloClient({
+    cache: new InMemoryCache({
+      typePolicies: {
+        Query: {
+          fields: {
+            letters: { keyArgs: false },
+          },
+        },
+      },
+    }),
+    link,
+  });
 
   const Profiler = createProfiler({
     initialSnapshot: {
@@ -1589,7 +1622,18 @@ test("`fetchMore` works with startTransition", async () => {
   const { query, link } = setupPaginatedCase();
 
   const user = userEvent.setup();
-  const client = new ApolloClient({ cache: new InMemoryCache(), link });
+  const client = new ApolloClient({
+    cache: new InMemoryCache({
+      typePolicies: {
+        Query: {
+          fields: {
+            letters: { keyArgs: false },
+          },
+        },
+      },
+    }),
+    link,
+  });
   const preloadQuery = createQueryPreloader(client);
 
   const Profiler = createProfiler({
@@ -1708,7 +1752,18 @@ test("`fetchMore` works with startTransition from useBackgroundQuery and useQuer
   const { query, link } = setupPaginatedCase();
 
   const user = userEvent.setup();
-  const client = new ApolloClient({ cache: new InMemoryCache(), link });
+  const client = new ApolloClient({
+    cache: new InMemoryCache({
+      typePolicies: {
+        Query: {
+          fields: {
+            letters: { keyArgs: false },
+          },
+        },
+      },
+    }),
+    link,
+  });
 
   const Profiler = createProfiler({
     initialSnapshot: {

diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx
@@ -10109,6 +10109,150 @@ describe("useSuspenseQuery", () => {
     await expect(Profiler).not.toRerender();
   });
 
+  // https://github.com/apollographql/apollo-client/issues/11642
+  it("returns merged array when `fetchMore` returns empty array of results", async () => {
+    const query: TypedDocumentNode<PaginatedCaseData, PaginatedCaseVariables> =
+      gql`
+        query LettersQuery($limit: Int, $offset: Int) {
+          letters(limit: $limit, offset: $offset) {
+            letter
+            position
+          }
+        }
+      `;
+
+    const data = "ABCD".split("").map((letter, index) => ({
+      __typename: "Letter",
+      letter,
+      position: index + 1,
+    }));
+
+    const link = new MockLink([
+      {
+        request: { query, variables: { offset: 0, limit: 2 } },
+        result: { data: { letters: data.slice(0, 2) } },
+        delay: 20,
+      },
+      {
+        request: { query, variables: { offset: 2, limit: 2 } },
+        result: { data: { letters: data.slice(2, 4) } },
+        delay: 20,
+      },
+      {
+        request: { query, variables: { offset: 4, limit: 2 } },
+        result: { data: { letters: [] } },
+        delay: 20,
+      },
+    ]);
+
+    const user = userEvent.setup();
+    const client = new ApolloClient({
+      cache: new InMemoryCache({
+        typePolicies: {
+          Query: {
+            fields: {
+              letters: offsetLimitPagination(),
+            },
+          },
+        },
+      }),
+      link,
+    });
+
+    const Profiler = createProfiler({
+      initialSnapshot: {
+        result: null as UseSuspenseQueryResult<
+          PaginatedCaseData,
+          PaginatedCaseVariables
+        > | null,
+      },
+    });
+
+    function App() {
+      useTrackRenders();
+      const result = useSuspenseQuery(query, {
+        variables: { offset: 0, limit: 2 },
+      });
+      const { data, fetchMore } = result;
+
+      Profiler.mergeSnapshot({ result });
+
+      return (
+        <button
+          onClick={() =>
+            fetchMore({
+              variables: {
+                offset: data.letters.length,
+                limit: 2,
+              },
+            })
+          }
+        >
+          Fetch next
+        </button>
+      );
+    }
+
+    render(<App />, {
+      wrapper: ({ children }) => (
+        <ApolloProvider client={client}>
+          <Profiler>
+            <Suspense fallback={<div>Loading...</div>}>{children}</Suspense>
+          </Profiler>
+        </ApolloProvider>
+      ),
+    });
+
+    // initial suspended render
+    await Profiler.takeRender();
+
+    {
+      const { snapshot, renderedComponents } = await Profiler.takeRender();
+
+      expect(renderedComponents).toStrictEqual([App]);
+      expect(snapshot.result?.data).toEqual({
+        letters: [
+          { __typename: "Letter", letter: "A", position: 1 },
+          { __typename: "Letter", letter: "B", position: 2 },
+        ],
+      });
+    }
+
+    await act(() => user.click(screen.getByText("Fetch next")));
+    await Profiler.takeRender();
+
+    {
+      const { snapshot } = await Profiler.takeRender();
+
+      expect(snapshot.result?.data).toEqual({
+        letters: [
+          { __typename: "Letter", letter: "A", position: 1 },
+          { __typename: "Letter", letter: "B", position: 2 },
+          { __typename: "Letter", letter: "C", position: 3 },
+          { __typename: "Letter", letter: "D", position: 4 },
+        ],
+      });
+    }
+
+    await act(() => user.click(screen.getByText("Fetch next")));
+    await Profiler.takeRender();
+
+    {
+      const { snapshot } = await Profiler.takeRender();
+
+      expect(snapshot.result?.data).toEqual({
+        letters: [
+          { __typename: "Letter", letter: "A", position: 1 },
+          { __typename: "Letter", letter: "B", position: 2 },
+          { __typename: "Letter", letter: "C", position: 3 },
+          { __typename: "Letter", letter: "D", position: 4 },
+        ],
+      });
+    }
+
+    await expect(Profiler).not.toRerender();
+  });
+
   describe.skip("type tests", () => {
     it("returns unknown when TData cannot be inferred", () => {
       const query = gql`

diff --git a/src/react/internal/cache/QueryReference.ts b/src/react/internal/cache/QueryReference.ts
@@ -377,7 +377,7 @@ export class InternalQueryReference<TData = unknown> {
     // to resolve the promise if `handleNext` hasn't been run to ensure the
     // promise is resolved correctly.
     returnedPromise
-      .then((result) => {
+      .then(() => {
         // In the case of `fetchMore`, this promise is resolved before a cache
         // result is emitted due to the fact that `fetchMore` sets a `no-cache`
         // fetch policy and runs `cache.batch` in its `.then` handler. Because
@@ -390,8 +390,16 @@ export class InternalQueryReference<TData = unknown> {
         // more information
         setTimeout(() => {
           if (this.promise.status === "pending") {
-            this.result = result;
-            this.resolve?.(result);
+            // Use the current result from the observable instead of the value
+            // resolved from the promise. This avoids issues in some cases where
+            // the raw resolved value should not be the emitted value, such as
+            // when a `fetchMore` call returns an empty array after it has
+            // reached the end of the list.
+            //
+            // See the following for more information:
+            // https://github.com/apollographql/apollo-client/issues/11642
+            this.result = this.observable.getCurrentResult();
+            this.resolve?.(this.result);
           }
         });
       })