Fix auto dispose timeout that could cause components to suspend indef…

…initely (#11274)

The `autoDisposeTimeoutMs` configurable value for Suspense hooks was created to ensure components that suspend but never remount would get properly cleaned up after the timeout duration. This works well to cleanup after components that never mount again after initially suspending (such as a user interaction that hides the element while suspended). This timer started when the query ref was initialized and disposed of regardless of whether the initial promise had settled or not. This meant that when the request finished after the configured timeout, the component would suspend indefinitely since nothing was around to resolve/reject the promise leaving it in a pending state forever.

This change adjusts the start of the dispose timer to wait until the initial promise is settled. This ensures React will try rendering again regardless of how long the request takes, and only after `autoDisposeTimeoutMs` will the query ref try and clean itself up if a component doesn't come along and retain it.

.changeset/lemon-tigers-promise.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Start the query ref auto dispose timeout after the initial promise has settled. This prevents requests that run longer than the timeout duration from keeping the component suspended indefinitely.

.size-limit.cjs

@@ -1,7 +1,7 @@
 const checks = [
   {
     path: "dist/apollo-client.min.cjs",
-    limit: "37940",
+    limit: "37960",
   },
   {
     path: "dist/main.cjs",

src/react/cache/QueryReference.ts

@@ -123,10 +123,19 @@ export class InternalQueryReference<TData = unknown> {
     // suspended resource does not use this queryRef in the given time. This
     // helps prevent memory leaks when a component has unmounted before the
     // query has finished loading.
-    this.autoDisposeTimeoutId = setTimeout(
-      this.dispose,
-      options.autoDisposeTimeoutMs ?? 30_000
-    );
+    const startDisposeTimer = () => {
+      if (!this.references) {
+        this.autoDisposeTimeoutId = setTimeout(
+          this.dispose,
+          options.autoDisposeTimeoutMs ?? 30_000
+        );
+      }
+    };
+
+    // We wait until the request has settled to ensure we don't dispose of the
+    // query ref before the request finishes, otherwise we would leave the
+    // promise in a pending state rendering the suspense boundary indefinitely.
+    this.promise.then(startDisposeTimer, startDisposeTimer);
   }
 
   get watchQueryOptions() {

src/react/hooks/__tests__/useSuspenseQuery.test.tsx

@@ -805,8 +805,12 @@ describe("useSuspenseQuery", () => {
 
     expect(screen.queryByText("Loading greeting...")).not.toBeInTheDocument();
 
-    link.simulateResult({ result: { data: { greeting: "Hello" } } });
-    link.simulateComplete();
+    await act(() => {
+      link.simulateResult({ result: { data: { greeting: "Hello" } } }, true);
+      // Ensure simulateResult will deliver the result since its wrapped with
+      // setTimeout
+      jest.advanceTimersByTime(10);
+    });
 
     expect(client.getObservableQueries().size).toBe(1);
     expect(client).toHaveSuspenseCacheEntryUsing(query);
@@ -817,10 +821,6 @@ describe("useSuspenseQuery", () => {
     expect(client).not.toHaveSuspenseCacheEntryUsing(query);
 
     jest.useRealTimers();
-
-    // Avoid act warnings for a suspended resource
-    // eslint-disable-next-line testing-library/no-unnecessary-act
-    await act(() => wait(0));
   });
 
   it("has configurable auto dispose timer if the component never renders again after suspending", async () => {
@@ -871,8 +871,12 @@ describe("useSuspenseQuery", () => {
 
     expect(screen.queryByText("Loading greeting...")).not.toBeInTheDocument();
 
-    link.simulateResult({ result: { data: { greeting: "Hello" } } });
-    link.simulateComplete();
+    await act(() => {
+      link.simulateResult({ result: { data: { greeting: "Hello" } } }, true);
+      // Ensure simulateResult will deliver the result since its wrapped with
+      // setTimeout
+      jest.advanceTimersByTime(10);
+    });
 
     expect(client.getObservableQueries().size).toBe(1);
     expect(client).toHaveSuspenseCacheEntryUsing(query);
@@ -883,10 +887,6 @@ describe("useSuspenseQuery", () => {
     expect(client).not.toHaveSuspenseCacheEntryUsing(query);
 
     jest.useRealTimers();
-
-    // Avoid act warnings for a suspended resource
-    // eslint-disable-next-line testing-library/no-unnecessary-act
-    await act(() => wait(0));
   });
 
   it("cancels auto dispose if the component renders before timer finishes", async () => {
@@ -940,6 +940,57 @@ describe("useSuspenseQuery", () => {
     jest.useRealTimers();
   });
 
+  // https://github.com/apollographql/apollo-client/issues/11270
+  it("does not leave component suspended if query completes if request takes longer than auto dispose timeout", async () => {
+    jest.useFakeTimers();
+    const { query } = useSimpleQueryCase();
+    const link = new MockSubscriptionLink();
+    const client = new ApolloClient({
+      link,
+      cache: new InMemoryCache(),
+      defaultOptions: {
+        react: {
+          suspense: {
+            autoDisposeTimeoutMs: 10,
+          },
+        },
+      },
+    });
+
+    function App() {
+      return (
+        <ApolloProvider client={client}>
+          <Suspense fallback="Loading greeting...">
+            <Greeting />
+          </Suspense>
+        </ApolloProvider>
+      );
+    }
+
+    function Greeting() {
+      const { data } = useSuspenseQuery(query);
+
+      return <span>{data.greeting}</span>;
+    }
+
+    render(<App />);
+
+    // Ensure <Greeting /> suspends immediately
+    expect(screen.getByText("Loading greeting...")).toBeInTheDocument();
+
+    jest.advanceTimersByTime(20);
+
+    link.simulateResult({ result: { data: { greeting: "Hello" } } }, true);
+
+    await waitFor(() => {
+      expect(screen.queryByText("Loading greeting...")).not.toBeInTheDocument();
+    });
+
+    expect(screen.getByText("Hello")).toBeInTheDocument();
+
+    jest.useRealTimers();
+  });
+
   it("allows the client to be overridden", async () => {
     const { query } = useSimpleQueryCase();