Skip to content

Commit

Permalink
fix useBackgroundQuery: dispose ref after unmount and not used (#11696)
Browse files Browse the repository at this point in the history
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
  • Loading branch information
PiR1 and jerelmiller authored Mar 21, 2024
1 parent 8e5c66b commit 466ef82
Show file tree
Hide file tree
Showing 9 changed files with 209 additions and 2 deletions.
2 changes: 2 additions & 0 deletions .api-reports/api-report-react.md
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,8 @@ class InternalQueryReference<TData = unknown> {
// (undocumented)
retain(): () => void;
// (undocumented)
softRetain(): () => void;
// (undocumented)
get watchQueryOptions(): WatchQueryOptions<OperationVariables, TData>;
}

Expand Down
2 changes: 2 additions & 0 deletions .api-reports/api-report-react_hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,8 @@ class InternalQueryReference<TData = unknown> {
// (undocumented)
retain(): () => void;
// (undocumented)
softRetain(): () => void;
// (undocumented)
get watchQueryOptions(): WatchQueryOptions<OperationVariables, TData>;
}

Expand Down
2 changes: 2 additions & 0 deletions .api-reports/api-report-react_internal.md
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,8 @@ export class InternalQueryReference<TData = unknown> {
// (undocumented)
retain(): () => void;
// (undocumented)
softRetain(): () => void;
// (undocumented)
get watchQueryOptions(): WatchQueryOptions<OperationVariables, TData>;
}

Expand Down
2 changes: 2 additions & 0 deletions .api-reports/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,8 @@ class InternalQueryReference<TData = unknown> {
// (undocumented)
retain(): () => void;
// (undocumented)
softRetain(): () => void;
// (undocumented)
get watchQueryOptions(): WatchQueryOptions<OperationVariables, TData>;
}

Expand Down
5 changes: 5 additions & 0 deletions .changeset/fast-roses-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Immediately dispose of the `queryRef` if `useBackgroundQuery` unmounts before the auto dispose timeout kicks in.
2 changes: 1 addition & 1 deletion .size-limits.json
Original file line number Diff line number Diff line change
@@ -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
}
169 changes: 169 additions & 0 deletions src/react/hooks/__tests__/useBackgroundQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ function createErrorProfiler<TData = unknown>() {
},
});
}

function createDefaultProfiler<TData = unknown>() {
return createProfiler({
initialSnapshot: {
Expand Down Expand Up @@ -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<SimpleCaseData>();

function App() {
useTrackRenders();
useBackgroundQuery(query);

return null;
}

const { unmount } = renderWithClient(<App />, { 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<SimpleCaseData>();

function App({ id }: { id: string }) {
useTrackRenders();
useBackgroundQuery(query, { variables: { id } });

return null;
}

const { rerender } = renderWithClient(<App id="1" />, {
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(<App id="2" />);

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<SimpleCaseData>();

function App() {
useTrackRenders();
useBackgroundQuery(query);

return null;
}

renderWithClient(<App />, {
client,
wrapper: ({ children }) => (
<React.StrictMode>
<Profiler>{children}</Profiler>
</React.StrictMode>
),
});

{
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<SimpleCaseData>();

function App() {
useTrackRenders();
useBackgroundQuery(query);

const [a, setA] = React.useState(0);

return (
<>
<button onClick={() => setA(a + 1)}>Increment</button>
</>
);
}

const { unmount } = renderWithClient(<App />, {
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();

Expand Down Expand Up @@ -985,6 +1149,7 @@ it("works with startTransition to change variables", async () => {
completed: boolean;
};
}

const user = userEvent.setup();

const query: TypedDocumentNode<Data, Variables> = gql`
Expand Down Expand Up @@ -4189,6 +4354,7 @@ describe("refetch", () => {
completed: boolean;
};
}

const user = userEvent.setup();

const query: TypedDocumentNode<Data, Variables> = gql`
Expand Down Expand Up @@ -4437,6 +4603,7 @@ describe("refetch", () => {
completed: boolean;
};
}

const user = userEvent.setup();

const query: TypedDocumentNode<Data, Variables> = gql`
Expand Down Expand Up @@ -5055,9 +5222,11 @@ describe("fetchMore", () => {
name: string;
completed: boolean;
}

interface Data {
todos: Todo[];
}

const user = userEvent.setup();

const query: TypedDocumentNode<Data, Variables> = gql`
Expand Down
4 changes: 3 additions & 1 deletion src/react/hooks/useBackgroundQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -261,6 +261,8 @@ function _useBackgroundQuery<
[queryRef]
);

React.useEffect(() => queryRef.softRetain(), [queryRef]);

return [
didFetchResult.current ? wrappedQueryRef : void 0,
{ fetchMore, refetch },
Expand Down
23 changes: 23 additions & 0 deletions src/react/internal/cache/QueryReference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ export class InternalQueryReference<TData = unknown> {
private reject: ((error: unknown) => void) | undefined;

private references = 0;
private softReferences = 0;

constructor(
observable: ObservableQuery<TData, any>,
Expand Down Expand Up @@ -250,6 +251,28 @@ export class InternalQueryReference<TData = unknown> {
};
}

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) =>
Expand Down

0 comments on commit 466ef82

Please sign in to comment.