From 2ae911b7eaa21231e54266681be43c484f6425bc Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 19 Mar 2024 14:32:14 -0600 Subject: [PATCH 1/7] Create failing test for fetchMore --- .../hooks/__tests__/useSuspenseQuery.test.tsx | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index 4e32f276c72..9a0e61fd55f 100644 --- 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 = + 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 ( + + ); + } + + render(, { + wrapper: ({ children }) => ( + + + Loading...}>{children} + + + ), + }); + + // 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` From 69915b566c2287ec782fee77fa16fa55142ae8fd Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 19 Mar 2024 15:08:24 -0600 Subject: [PATCH 2/7] Use getCurrentResult to set the value in QueryReference instead of the resolved value --- src/react/internal/cache/QueryReference.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/react/internal/cache/QueryReference.ts b/src/react/internal/cache/QueryReference.ts index 83b4f20ac49..110a0823bb8 100644 --- a/src/react/internal/cache/QueryReference.ts +++ b/src/react/internal/cache/QueryReference.ts @@ -377,7 +377,7 @@ export class InternalQueryReference { // 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 { // 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); } }); }) From 1d9be5f3341217fdc57e36e398bac84dd3fd52ab Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 19 Mar 2024 15:08:47 -0600 Subject: [PATCH 3/7] Update paginated test to define type policy --- .../hooks/__tests__/useLoadableQuery.test.tsx | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/react/hooks/__tests__/useLoadableQuery.test.tsx b/src/react/hooks/__tests__/useLoadableQuery.test.tsx index 68ef6a7e9a7..4001dea7bd6 100644 --- 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(); 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, From e562b3c834e449a988741f1ca1f0f065ce710270 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 19 Mar 2024 15:11:30 -0600 Subject: [PATCH 4/7] Add type policy for failing test in useBackgroundQuery --- src/react/hooks/__tests__/useBackgroundQuery.test.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx index ac7477d51f7..adcc9f430d2 100644 --- 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(); const { SuspenseFallback, ReadQueryHook } = @@ -4818,7 +4827,7 @@ describe("fetchMore", () => { ); } - renderWithMocks(, { link, wrapper: Profiler }); + renderWithMocks(, { cache, link, wrapper: Profiler }); { const { renderedComponents } = await Profiler.takeRender(); From 391cb947b933e195a12c2836aaa049b7b8013b9f Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 19 Mar 2024 15:16:27 -0600 Subject: [PATCH 5/7] Update failing tests in useQueryRefHandlers --- .../__tests__/useQueryRefHandlers.test.tsx | 65 +++++++++++++++++-- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/src/react/hooks/__tests__/useQueryRefHandlers.test.tsx b/src/react/hooks/__tests__/useQueryRefHandlers.test.tsx index a7f2dd72f43..a81c64d8373 100644 --- 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: { From 00a0508e2782144b22b65e9dc864ba0abb9fad63 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 19 Mar 2024 15:18:37 -0600 Subject: [PATCH 6/7] Add changeset --- .changeset/curly-berries-hammer.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/curly-berries-hammer.md diff --git a/.changeset/curly-berries-hammer.md b/.changeset/curly-berries-hammer.md new file mode 100644 index 00000000000..7b05ebad124 --- /dev/null +++ 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. From e43dbd90946c029dc95c5ee8a9659df09463b81f Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 19 Mar 2024 15:33:11 -0600 Subject: [PATCH 7/7] Update size limits --- .size-limits.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limits.json b/.size-limits.json index 6ea6a4c6ebc..2c7dbd56314 100644 --- 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 }