From 466ef82198486fc696da64d17d82b46140760ac4 Mon Sep 17 00:00:00 2001 From: PiR Date: Thu, 21 Mar 2024 23:57:48 +0100 Subject: [PATCH] fix useBackgroundQuery: dispose ref after unmount and not used (#11696) Co-authored-by: Jerel Miller --- .api-reports/api-report-react.md | 2 + .api-reports/api-report-react_hooks.md | 2 + .api-reports/api-report-react_internal.md | 2 + .api-reports/api-report.md | 2 + .changeset/fast-roses-kick.md | 5 + .size-limits.json | 2 +- .../__tests__/useBackgroundQuery.test.tsx | 169 ++++++++++++++++++ src/react/hooks/useBackgroundQuery.ts | 4 +- src/react/internal/cache/QueryReference.ts | 23 +++ 9 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 .changeset/fast-roses-kick.md diff --git a/.api-reports/api-report-react.md b/.api-reports/api-report-react.md index d604f535089..a12089d47fc 100644 --- a/.api-reports/api-report-react.md +++ b/.api-reports/api-report-react.md @@ -953,6 +953,8 @@ class InternalQueryReference { // (undocumented) retain(): () => void; // (undocumented) + softRetain(): () => void; + // (undocumented) get watchQueryOptions(): WatchQueryOptions; } diff --git a/.api-reports/api-report-react_hooks.md b/.api-reports/api-report-react_hooks.md index e87875b811a..8e157061ac6 100644 --- a/.api-reports/api-report-react_hooks.md +++ b/.api-reports/api-report-react_hooks.md @@ -895,6 +895,8 @@ class InternalQueryReference { // (undocumented) retain(): () => void; // (undocumented) + softRetain(): () => void; + // (undocumented) get watchQueryOptions(): WatchQueryOptions; } diff --git a/.api-reports/api-report-react_internal.md b/.api-reports/api-report-react_internal.md index 84afcfd091a..d81159f41cd 100644 --- a/.api-reports/api-report-react_internal.md +++ b/.api-reports/api-report-react_internal.md @@ -885,6 +885,8 @@ export class InternalQueryReference { // (undocumented) retain(): () => void; // (undocumented) + softRetain(): () => void; + // (undocumented) get watchQueryOptions(): WatchQueryOptions; } diff --git a/.api-reports/api-report.md b/.api-reports/api-report.md index ebbea8fb589..6d1e721d379 100644 --- a/.api-reports/api-report.md +++ b/.api-reports/api-report.md @@ -1328,6 +1328,8 @@ class InternalQueryReference { // (undocumented) retain(): () => void; // (undocumented) + softRetain(): () => void; + // (undocumented) get watchQueryOptions(): WatchQueryOptions; } diff --git a/.changeset/fast-roses-kick.md b/.changeset/fast-roses-kick.md new file mode 100644 index 00000000000..592ee93e736 --- /dev/null +++ b/.changeset/fast-roses-kick.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Immediately dispose of the `queryRef` if `useBackgroundQuery` unmounts before the auto dispose timeout kicks in. diff --git a/.size-limits.json b/.size-limits.json index 10e91f0ebf7..3dc2aae6853 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 39277, + "dist/apollo-client.min.cjs": 39319, "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 index 045f7221e7a..198e3601aa3 100644 --- a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx +++ b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx @@ -111,6 +111,7 @@ function createErrorProfiler() { }, }); } + function createDefaultProfiler() { return createProfiler({ initialSnapshot: { @@ -446,6 +447,169 @@ it("auto resubscribes when mounting useReadQuery after naturally disposed by use await expect(Profiler).not.toRerender({ timeout: 50 }); }); +it("disposes of the queryRef when unmounting before it is used by useReadQuery", async () => { + const { query, mocks } = setupSimpleCase(); + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + const Profiler = createDefaultProfiler(); + + function App() { + useTrackRenders(); + useBackgroundQuery(query); + + return null; + } + + const { unmount } = renderWithClient(, { client, wrapper: Profiler }); + + expect(client.getObservableQueries().size).toBe(1); + expect(client).toHaveSuspenseCacheEntryUsing(query); + + { + const { renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + } + + unmount(); + await wait(0); + + expect(client.getObservableQueries().size).toBe(0); + expect(client).not.toHaveSuspenseCacheEntryUsing(query); +}); + +it("disposes of old queryRefs when changing variables before the queryRef is used by useReadQuery", async () => { + const { query, mocks } = setupVariablesCase(); + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + const Profiler = createDefaultProfiler(); + + function App({ id }: { id: string }) { + useTrackRenders(); + useBackgroundQuery(query, { variables: { id } }); + + return null; + } + + const { rerender } = renderWithClient(, { + client, + wrapper: Profiler, + }); + + expect(client.getObservableQueries().size).toBe(1); + expect(client).toHaveSuspenseCacheEntryUsing(query, { + variables: { id: "1" }, + }); + + { + const { renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + } + + rerender(); + + await wait(0); + + expect(client.getObservableQueries().size).toBe(1); + expect(client).toHaveSuspenseCacheEntryUsing(query, { + variables: { id: "2" }, + }); + expect(client).not.toHaveSuspenseCacheEntryUsing(query, { + variables: { id: "1" }, + }); +}); + +it("does not prematurely dispose of the queryRef when using strict mode", async () => { + const { query, mocks } = setupSimpleCase(); + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + const Profiler = createDefaultProfiler(); + + function App() { + useTrackRenders(); + useBackgroundQuery(query); + + return null; + } + + renderWithClient(, { + client, + wrapper: ({ children }) => ( + + {children} + + ), + }); + + { + const { renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + } + + await wait(10); + + expect(client.getObservableQueries().size).toBe(1); + expect(client).toHaveSuspenseCacheEntryUsing(query); +}); + +it("disposes of the queryRef when unmounting before it is used by useReadQuery even if it has been rerendered", async () => { + const { query, mocks } = setupSimpleCase(); + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + const user = userEvent.setup(); + + const Profiler = createDefaultProfiler(); + + function App() { + useTrackRenders(); + useBackgroundQuery(query); + + const [a, setA] = React.useState(0); + + return ( + <> + + + ); + } + + const { unmount } = renderWithClient(, { + client, + wrapper: Profiler, + }); + const button = screen.getByText("Increment"); + + await act(() => user.click(button)); + + { + const { renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + } + + expect(client.getObservableQueries().size).toBe(1); + expect(client).toHaveSuspenseCacheEntryUsing(query); + + await wait(0); + + unmount(); + await wait(0); + expect(client.getObservableQueries().size).toBe(0); +}); + it("allows the client to be overridden", async () => { const { query } = setupSimpleCase(); @@ -985,6 +1149,7 @@ it("works with startTransition to change variables", async () => { completed: boolean; }; } + const user = userEvent.setup(); const query: TypedDocumentNode = gql` @@ -4189,6 +4354,7 @@ describe("refetch", () => { completed: boolean; }; } + const user = userEvent.setup(); const query: TypedDocumentNode = gql` @@ -4437,6 +4603,7 @@ describe("refetch", () => { completed: boolean; }; } + const user = userEvent.setup(); const query: TypedDocumentNode = gql` @@ -5055,9 +5222,11 @@ describe("fetchMore", () => { name: string; completed: boolean; } + interface Data { todos: Todo[]; } + const user = userEvent.setup(); const query: TypedDocumentNode = gql` diff --git a/src/react/hooks/useBackgroundQuery.ts b/src/react/hooks/useBackgroundQuery.ts index 8d83c50fc00..ba5f8c19221 100644 --- a/src/react/hooks/useBackgroundQuery.ts +++ b/src/react/hooks/useBackgroundQuery.ts @@ -15,7 +15,7 @@ import { } from "../internal/index.js"; import type { CacheKey, QueryReference } from "../internal/index.js"; import type { BackgroundQueryHookOptions, NoInfer } from "../types/types.js"; -import { __use, wrapHook } from "./internal/index.js"; +import { wrapHook } from "./internal/index.js"; import { useWatchQueryOptions } from "./useSuspenseQuery.js"; import type { FetchMoreFunction, RefetchFunction } from "./useSuspenseQuery.js"; import { canonicalStringify } from "../../cache/index.js"; @@ -261,6 +261,8 @@ function _useBackgroundQuery< [queryRef] ); + React.useEffect(() => queryRef.softRetain(), [queryRef]); + return [ didFetchResult.current ? wrappedQueryRef : void 0, { fetchMore, refetch }, diff --git a/src/react/internal/cache/QueryReference.ts b/src/react/internal/cache/QueryReference.ts index 68efe9272b7..c40a155a28f 100644 --- a/src/react/internal/cache/QueryReference.ts +++ b/src/react/internal/cache/QueryReference.ts @@ -158,6 +158,7 @@ export class InternalQueryReference { private reject: ((error: unknown) => void) | undefined; private references = 0; + private softReferences = 0; constructor( observable: ObservableQuery, @@ -250,6 +251,28 @@ export class InternalQueryReference { }; } + softRetain() { + this.softReferences++; + let disposed = false; + + return () => { + // Tracking if this has already been called helps ensure that + // multiple calls to this function won't decrement the reference + // counter more than it should. Subsequent calls just result in a noop. + if (disposed) { + return; + } + + disposed = true; + this.softReferences--; + setTimeout(() => { + if (!this.softReferences && !this.references) { + this.dispose(); + } + }); + }; + } + didChangeOptions(watchQueryOptions: ObservedOptions) { return OBSERVED_CHANGED_OPTIONS.some( (option) =>