useQuery refetch does not re-render for new variables if response remains the same

[maciesielka]

trafficstars

Issue Description

Library Version: v3.8.6

There seems to be an optimization in place to prevent redundant re-renders when useQuery produces the same response, but I believe that same code path is inadvertently preventing a re-render when that hook's refetch passes a new set of variables to execute a new query. This seems undesirable because the variables returned by the hook may fall out of sync with the actually executed request.

While it's easy enough to work around this and store the variables in a separate state, it seems inconsistent for the variables returned by the hook to sometimes reference the most recently run query and sometimes not.

I think the following diff will add a failing test to the existing suite:

Failing test

diff --git a/src/react/hoc/__tests__/queries/observableQuery.test.tsx b/src/react/hoc/__tests__/queries/observableQuery.test.tsx
index dd05663aa..88cc39ddf 100644
--- a/src/react/hoc/__tests__/queries/observableQuery.test.tsx
+++ b/src/react/hoc/__tests__/queries/observableQuery.test.tsx
@@ -472,4 +472,94 @@ describe("[queries] observableQuery", () => {
       return waitFor(() => expect(count).toBe(3)).then(resolve, reject);
     }
   );
+
+  itAsync(
+    "does rerender if query returns same result for different variables",
+    (resolve, reject) => {
+      const query: DocumentNode = gql`
+        query people($first: Int!) {
+          allPeople(first: $first) {
+            people {
+              name
+              friends(id: $first) {
+                name
+              }
+            }
+          }
+        }
+      `;
+
+      const variables1 = { first: 1 };
+      const variables2 = { first: 2 };
+      const data = {
+        allPeople: {
+          people: [{ name: "Luke Skywalker", friends: [{ name: "r2d2" }] }],
+        },
+      };
+
+      type Data = typeof data;
+      type Vars = { first: number };
+
+      const link = mockSingleLink(
+        {
+          request: { query, variables: variables1 },
+          result: { data },
+        },
+        {
+          request: { query, variables: variables2 },
+          result: { data },
+        }
+      );
+
+      const client = new ApolloClient({
+        link,
+        cache: new Cache({ addTypename: false }),
+      });
+
+      let count = 0;
+      const Container = graphql<Vars, Data, Vars>(query)(
+        class extends React.Component<ChildProps<Vars, Data, Vars>> {
+          render() {
+            count++;
+            try {
+              const { loading, allPeople, refetch, variables } =
+                this.props.data!;
+              // first variable render
+              if (count === 1) {
+                expect(loading).toBe(true);
+              }
+              if (count === 2) {
+                expect(loading).toBe(false);
+                expect(variables).toEqual(variables1);
+                expect(allPeople).toEqual(data.allPeople);
+                refetch({ ...variables2 });
+              }
+              if (count === 3) {
+                expect(loading).toBe(false);
+                expect(variables).toEqual(variables2);
+                expect(allPeople).toEqual(data.allPeople);
+              }
+              if (count > 3) {
+                throw new Error("too many renders");
+              }
+            } catch (e) {
+              reject(e);
+            }
+
+            return null;
+          }
+        }
+      );
+
+      // the initial mount fires off the query
+      // the same as episode id = 1
+      render(
+        <ApolloProvider client={client}>
+          <Container first={1} />
+        </ApolloProvider>
+      );
+
+      return waitFor(() => expect(count).toBe(3)).then(resolve, reject);
+    }
+  );
 });

Link to Reproduction

CodeSandbox

Reproduction Steps

Using the Sandbox example, attempt the following steps:

1. Focus on the search field
2. Begin typing the name "John", one letter a time. Observe that the list properly filters as you type to just show "John Smith".
3. Build on this search to query for "Johnny", again one letter a time.
   1. Type "n" and observe an error message appears showing "Johnn" as the query that returns no results
   2. Type "y"

Observe: the view never re-renders to show an error message with "Johnny" as the query that returns no results Expected: the view should re-render after receiving a response to show an error message with "Johnny" as the query that returns no results

The issue appears to be that since both steps 3i and 3ii produce the same response, no render is issued.

[bignimbus]

Thanks for opening this issue @maciesielka! I've put it on the agenda for an upcoming team meeting, we'll let you know once we've had a chance to triage a bit more!

[bignimbus]

Okay I have an update for you @maciesielka! This is definitely an issue we'd want to fix but we also need to be careful not to introduce a performance regression, so I don't expect that we'd be able to prioritize this in the near future. As a workaround, consider notifyOnNetworkStatusChange: true as an option for useQuery.

Also, I wanted to express our gratitude for submitting a failing test. Though the hoc directory isn't the place where we would place such a test, the logic itself helped us reason about your request 🙏🏻

[0x7aF777]

It seems it is because of this change -- https://github.com/apollographql/apollo-client/commit/4f73c5ca15d367aa23f02018d062f221c4506a4d. There are similar issue for polling and fetchMore as well.

[andrianarivo]

Hi ! Currently having the same issue here.

Is this fixed with new versions ? In the meantime, I did call refetch inside of useEffect to re-render the UI. (just for those in search for a workaround)

[tpict]

Any chance of this being resolved in 4.0?

[jerelmiller]

Hey @tpict 👋

We had fixed this issue in Apollo Client core for 4.0 so that a refetch that returns a deep equal result will ensure the observable emits a value when notifyOnNetworkStatusChange is false, but it looks like we didn't update useQuery to handle that fix. I just opened #12729 which should address this issue and will be released in 4.0 (likely in rc.1 if you want to try it out after that 🙂).

[tpict]

That’s great! Thanks kindly @jerelmiller

[jerelmiller]

Hey all 👋

We have just released this fix in 4.0.0-rc.1. If you'd be willing to try it out and provide feedback, that would be super helpful!

Since this has been completed and published in a prerelease, I'm going to go ahead and close out this issue. Thanks!

[github-actions[bot]]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. For general questions, we recommend using our Community Forum or Stack Overflow.