Ensure a refetch that changes variables and returns deep equal results rerenders (#12729)

Author: jerelmiller

.changeset/tough-olives-fry.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Ensure `useQuery` rerenders when `notifyOnNetworkStatusChange` is `false` and a `refetch` that changes variables returns a result deeply equal to previous variables.

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

@@ -13026,6 +13026,234 @@ test("renders loading states at appropriate times on next fetch after updating `
   await expect(takeSnapshot).not.toRerender();
 });
 
+// https://github.com/apollographql/apollo-client/issues/11328
+test("rerenders if refetch returns same result for different variables with notifyOnNetworkStatusChange: false", async () => {
+  type Data = typeof data;
+  type Vars = { first: number };
+  const query: TypedDocumentNode<Data, Vars> = gql`
+    query people($first: Int!) {
+      allPeople(first: $first) {
+        people {
+          name
+          friends(id: $first) {
+            name
+          }
+        }
+      }
+    }
+  `;
+
+  const variables1: Vars = { first: 1 };
+  const variables2: Vars = { first: 2 };
+  const data = {
+    allPeople: {
+      __typename: "AllPeople",
+      people: [
+        {
+          __typename: "Person",
+          name: "Luke Skywalker",
+          friends: [{ __typename: "Person", name: "r2d2" }],
+        },
+      ],
+    },
+  };
+
+  const link = new MockLink([
+    {
+      request: { query, variables: variables1 },
+      result: { data },
+      maxUsageCount: Number.POSITIVE_INFINITY,
+    },
+    {
+      request: { query, variables: variables2 },
+      result: { data },
+      maxUsageCount: Number.POSITIVE_INFINITY,
+    },
+  ]);
+
+  const client = new ApolloClient({
+    link,
+    cache: new InMemoryCache(),
+  });
+
+  using _disabledAct = disableActEnvironment();
+  const renderStream = await renderHookToSnapshotStream(
+    ({ variables }) =>
+      useQuery(query, { variables, notifyOnNetworkStatusChange: false }),
+    {
+      initialProps: { variables: variables1 },
+      wrapper: ({ children }) => (
+        <ApolloProvider client={client}>{children}</ApolloProvider>
+      ),
+    }
+  );
+
+  const { takeSnapshot, getCurrentSnapshot } = renderStream;
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data: undefined,
+    dataState: "empty",
+    loading: true,
+    networkStatus: NetworkStatus.loading,
+    previousData: undefined,
+    variables: variables1,
+  });
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data,
+    dataState: "complete",
+    loading: false,
+    networkStatus: NetworkStatus.ready,
+    previousData: undefined,
+    variables: variables1,
+  });
+
+  await expect(
+    getCurrentSnapshot().refetch(variables2)
+  ).resolves.toStrictEqualTyped({ data });
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data,
+    dataState: "complete",
+    loading: false,
+    networkStatus: NetworkStatus.ready,
+    previousData: undefined,
+    variables: variables2,
+  });
+
+  await expect(
+    getCurrentSnapshot().refetch(variables1)
+  ).resolves.toStrictEqualTyped({ data });
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data,
+    dataState: "complete",
+    loading: false,
+    networkStatus: NetworkStatus.ready,
+    previousData: undefined,
+    variables: variables1,
+  });
+
+  await expect(renderStream).not.toRerender();
+});
+
+test("rerenders if changing variables returns same result for different variables with notifyOnNetworkStatusChange: false", async () => {
+  type Data = typeof data;
+  type Vars = { first: number };
+  const query: TypedDocumentNode<Data, Vars> = gql`
+    query people($first: Int!) {
+      allPeople(first: $first) {
+        people {
+          name
+          friends(id: $first) {
+            name
+          }
+        }
+      }
+    }
+  `;
+
+  const variables1: Vars = { first: 1 };
+  const variables2: Vars = { first: 2 };
+  const data = {
+    allPeople: {
+      __typename: "AllPeople",
+      people: [
+        {
+          __typename: "Person",
+          name: "Luke Skywalker",
+          friends: [{ __typename: "Person", name: "r2d2" }],
+        },
+      ],
+    },
+  };
+
+  const link = new MockLink([
+    {
+      request: { query, variables: variables1 },
+      result: { data },
+      maxUsageCount: Number.POSITIVE_INFINITY,
+    },
+    {
+      request: { query, variables: variables2 },
+      result: { data },
+      maxUsageCount: Number.POSITIVE_INFINITY,
+    },
+  ]);
+
+  const client = new ApolloClient({
+    link,
+    cache: new InMemoryCache(),
+  });
+
+  using _disabledAct = disableActEnvironment();
+  const renderStream = await renderHookToSnapshotStream(
+    ({ variables }) =>
+      useQuery(query, { variables, notifyOnNetworkStatusChange: false }),
+    {
+      initialProps: { variables: variables1 },
+      wrapper: ({ children }) => (
+        <ApolloProvider client={client}>{children}</ApolloProvider>
+      ),
+    }
+  );
+
+  const { takeSnapshot, getCurrentSnapshot, rerender } = renderStream;
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data: undefined,
+    dataState: "empty",
+    loading: true,
+    networkStatus: NetworkStatus.loading,
+    previousData: undefined,
+    variables: variables1,
+  });
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data,
+    dataState: "complete",
+    loading: false,
+    networkStatus: NetworkStatus.ready,
+    previousData: undefined,
+    variables: variables1,
+  });
+
+  await rerender({ variables: variables2 });
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data: undefined,
+    dataState: "empty",
+    loading: true,
+    networkStatus: NetworkStatus.setVariables,
+    previousData: data,
+    variables: variables2,
+  });
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data,
+    dataState: "complete",
+    loading: false,
+    networkStatus: NetworkStatus.ready,
+    previousData: data,
+    variables: variables2,
+  });
+
+  await expect(
+    getCurrentSnapshot().refetch(variables1)
+  ).resolves.toStrictEqualTyped({ data });
+
+  await expect(takeSnapshot()).resolves.toStrictEqualTyped({
+    data,
+    dataState: "complete",
+    loading: false,
+    networkStatus: NetworkStatus.ready,
+    previousData: data,
+    variables: variables1,
+  });
+
+  await expect(renderStream).not.toRerender();
+});
+
 describe.skip("Type Tests", () => {
   test("returns narrowed TData in default case", () => {
     const { query } = setupSimpleCase();

src/react/hooks/useQuery.ts

@@ -174,6 +174,11 @@ interface InternalResult<TData> {
   // okay/normal for them to be initially undefined.
   current: ApolloQueryResult<TData>;
   previousData?: undefined | MaybeMasked<TData>;
+
+  // Track current variables separately in case a call to e.g. `refetch(newVars)`
+  // causes an emit that is deeply equal to the current result. This lets us
+  // compare if we should force rerender due to changed variables
+  variables: OperationVariables;
 }
 
 interface InternalState<TData, TVariables extends OperationVariables> {
@@ -311,6 +316,7 @@ function useQuery_<TData, TVariables extends OperationVariables>(
         // Reuse previousData from previous InternalState (if any) to provide
         // continuity of previousData even if/when the query or client changes.
         previousData: previous?.resultData.current.data as TData,
+        variables: observable.variables,
       },
     };
   }
@@ -405,13 +411,22 @@ function useResult<TData, TVariables extends OperationVariables>(
           .pipe(observeOn(asapScheduler))
           .subscribe((result) => {
             const previous = resultData.current;
-            // Make sure we're not attempting to re-render similar results
-            if (equal(previous, result)) {
+
+            if (
+              // Avoid rerendering if the result is the same
+              equal(previous, result) &&
+              // Force rerender if the value was emitted because variables
+              // changed, such as when calling `refetch(newVars)` which returns
+              // the same data when `notifyOnNetworkStatusChange` is `false`.
+              equal(resultData.variables, observable.variables)
+            ) {
               return;
             }
 
+            // eslint-disable-next-line react-compiler/react-compiler
+            resultData.variables = observable.variables;
+
             if (previous.data && !equal(previous.data, result.data)) {
-              // eslint-disable-next-line react-compiler/react-compiler
               resultData.previousData = previous.data as TData;
             }
 
@@ -476,6 +491,7 @@ function useResubscribeIfNecessary<
         (resultData.previousData as TData)) as TData;
     }
     resultData.current = result;
+    resultData.variables = observable.variables;
   }
   observable[lastWatchOptions] = watchQueryOptions;
 }