Update tests that check calls to internal QueryManager methods (#12573)

Author: jerelmiller

.size-limits.json

@@ -1,6 +1,6 @@
 {
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 43438,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 38878,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 33207,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 28023
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 43424,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 38812,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 33179,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 28041
 }

src/core/ObservableQuery.ts

@@ -1109,7 +1109,6 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
     // We want to ensure we can re-run the custom document transforms the next
     // time a request is made against the original query.
     const query = this.transformDocument(options.query);
-    const { fetchPolicy } = options;
 
     this.lastQuery = query;
 
@@ -1135,14 +1134,15 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
         newOptions.variables &&
         !equal(newOptions.variables, oldVariables) &&
         // Don't mess with the fetchPolicy if it's currently "standby".
-        fetchPolicy !== "standby" &&
+        options.fetchPolicy !== "standby" &&
         // If we're changing the fetchPolicy anyway, don't try to change it here
         // using applyNextFetchPolicy. The explicit options.fetchPolicy wins.
-        (fetchPolicy === oldFetchPolicy ||
+        (options.fetchPolicy === oldFetchPolicy ||
           // A `nextFetchPolicy` function has even higher priority, though,
           // so in that case `applyNextFetchPolicy` must be called.
           typeof options.nextFetchPolicy === "function")
       ) {
+        // This might mutate options.fetchPolicy
         this.applyNextFetchPolicy("variables-changed", options);
         if (newNetworkStatus === void 0) {
           newNetworkStatus = NetworkStatus.setVariables;
@@ -1165,12 +1165,12 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
 
       // QueryManager does not emit any values for standby fetch policies so we
       // want ensure that the networkStatus remains ready.
-      if (fetchPolicy === "standby") {
+      if (options.fetchPolicy === "standby") {
         newNetworkStatus = NetworkStatus.ready;
       }
     }
 
-    if (fetchPolicy === "standby") {
+    if (options.fetchPolicy === "standby") {
       this.cancelPolling();
     }
 

src/core/QueryManager.ts

@@ -1893,7 +1893,7 @@ function validateDidEmitValue<T>() {
 }
 
 // Return types used by fetchQueryByPolicy and other private methods above.
-export interface ObservableAndInfo<TData> {
+interface ObservableAndInfo<TData> {
   // Metadata properties that can be returned in addition to the Observable.
   fromLink: boolean;
   observable: Observable<ApolloQueryResult<TData>>;

src/core/__tests__/ObservableQuery.ts

@@ -28,25 +28,6 @@ import {
   removeDirectivesFromDocument,
 } from "@apollo/client/utilities";
 
-import type { ObservableAndInfo, QueryManager } from "../QueryManager.js";
-
-export const mockFetchQuery = (queryManager: QueryManager) => {
-  const mocks = {
-    fetchObservableWithInfo: jest.fn<
-      ObservableAndInfo<unknown>,
-      Parameters<QueryManager["fetchObservableWithInfo"]>
-    >(queryManager["fetchObservableWithInfo"].bind(queryManager)),
-    fetchQueryByPolicy: jest.fn<
-      ObservableAndInfo<unknown>,
-      Parameters<QueryManager["fetchQueryByPolicy"]>
-    >(queryManager["fetchQueryByPolicy"].bind(queryManager)),
-  };
-
-  Object.assign(queryManager, mocks);
-
-  return mocks;
-};
-
 describe("ObservableQuery", () => {
   // Standard data for all these tests
   const query: TypedDocumentNode<{
@@ -1597,34 +1578,29 @@ describe("ObservableQuery", () => {
   });
 
   describe("refetch", () => {
+    // TODO: If updating this to include a cache result for the changed
+    // variables, a cache value is emitted for the refetch, but I believe this
+    // should be undefined.
     it("calls fetchRequest with fetchPolicy `network-only` when using a non-networked fetch policy", async () => {
-      const mockedResponses = [
-        {
-          request: { query, variables },
-          result: { data: dataOne },
-          delay: 20,
-        },
-        {
-          request: { query, variables: differentVariables },
-          result: { data: dataTwo },
-          delay: 20,
-        },
-      ];
-
       const client = new ApolloClient({
         cache: new InMemoryCache(),
-        link: new MockLink(mockedResponses),
+        link: new MockLink([
+          {
+            request: { query, variables },
+            result: { data: dataOne },
+          },
+          {
+            request: { query, variables: differentVariables },
+            result: { data: dataTwo },
+          },
+        ]),
       });
-      const firstRequest = mockedResponses[0].request;
       const observable = client.watchQuery({
-        query: firstRequest.query,
-        variables: firstRequest.variables,
+        query,
+        variables,
         fetchPolicy: "cache-first",
       });
 
-      // TODO: Determine if we can test this without reaching into internal
-      // implementation details
-      const mocks = mockFetchQuery(client["queryManager"]);
       const stream = new ObservableStream(observable);
 
       await expect(stream).toEmitTypedValue({
@@ -1657,20 +1633,6 @@ describe("ObservableQuery", () => {
         partial: false,
       });
 
-      const fqbpCalls = mocks.fetchQueryByPolicy.mock.calls;
-      expect(fqbpCalls.length).toBe(2);
-      expect(fqbpCalls[0][1].fetchPolicy).toEqual("cache-first");
-      expect(fqbpCalls[1][1].fetchPolicy).toEqual("network-only");
-
-      const fqoCalls = mocks.fetchObservableWithInfo.mock.calls;
-      expect(fqoCalls.length).toBe(2);
-      expect(fqoCalls[0][1].fetchPolicy).toEqual("cache-first");
-      expect(fqoCalls[1][1].fetchPolicy).toEqual("network-only");
-
-      // Although the options.fetchPolicy we passed just now to
-      // fetchQueryByPolicy should have been network-only,
-      // observable.options.fetchPolicy should now be updated to
-      // cache-first, thanks to options.nextFetchPolicy.
       expect(observable.options.fetchPolicy).toBe("cache-first");
 
       await expect(stream).not.toEmitAnything();

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

@@ -44,7 +44,6 @@ import {
   MockLink,
   mockSingleLink,
   MockSubscriptionLink,
-  tick,
   wait,
 } from "@apollo/client/testing";
 import {
@@ -57,8 +56,6 @@ import type { Reference } from "@apollo/client/utilities";
 import { concatPagination } from "@apollo/client/utilities";
 import { InvariantError } from "@apollo/client/utilities/invariant";
 
-import type { QueryManager } from "../../../core/QueryManager.js";
-
 const IS_REACT_17 = React.version.startsWith("17");
 const IS_REACT_18 = React.version.startsWith("18");
 
@@ -7426,20 +7423,20 @@ describe("useQuery Hook", () => {
 
     it("should prioritize a `nextFetchPolicy` function over a `fetchPolicy` option when changing variables", async () => {
       const query = gql`
-        {
-          hello
+        query ($id: ID!) {
+          user(id: $id) {
+            id
+            name
+          }
         }
       `;
       const link = new MockLink([
         {
-          request: { query, variables: { id: 1 } },
-          result: { data: { hello: "from link" } },
-          delay: 10,
-        },
-        {
-          request: { query, variables: { id: 2 } },
-          result: { data: { hello: "from link2" } },
-          delay: 10,
+          request: { query, variables: () => true },
+          result: ({ id }) => ({
+            data: { user: { __typename: "User", id, name: `User ${id}` } },
+          }),
+          maxUsageCount: Number.POSITIVE_INFINITY,
         },
       ]);
 
@@ -7448,26 +7445,17 @@ describe("useQuery Hook", () => {
         link,
       });
 
-      const fetchQueryByPolicy = jest.spyOn(
-        client["queryManager"] as any as {
-          fetchQueryByPolicy: QueryManager["fetchQueryByPolicy"];
-        },
-        "fetchQueryByPolicy"
-      );
+      client.writeQuery({
+        query,
+        variables: { id: 1 },
+        data: { user: { __typename: "User", id: 1, name: "Cached User 1" } },
+      });
+      client.writeQuery({
+        query,
+        variables: { id: 2 },
+        data: { user: { __typename: "User", id: 2, name: "Cached User 2" } },
+      });
 
-      const expectQueryTriggered = (
-        nth: number,
-        fetchPolicy: WatchQueryFetchPolicy
-      ) => {
-        expect(fetchQueryByPolicy).toHaveBeenCalledTimes(nth);
-        expect(fetchQueryByPolicy).toHaveBeenNthCalledWith(
-          nth,
-          expect.anything(),
-          expect.objectContaining({ fetchPolicy }),
-          expect.any(Number),
-          expect.any(Boolean)
-        );
-      };
       const nextFetchPolicy: WatchQueryOptions<
         OperationVariables,
         any
@@ -7499,11 +7487,7 @@ describe("useQuery Hook", () => {
           }
         );
 
-      await tick();
-
-      // first network request triggers with initial fetchPolicy
-      expectQueryTriggered(1, "network-only");
-
+      // We skip the cache and go to the network
       await expect(takeSnapshot()).resolves.toStrictEqualTyped({
         data: undefined,
         loading: true,
@@ -7513,7 +7497,7 @@ describe("useQuery Hook", () => {
       });
 
       await expect(takeSnapshot()).resolves.toStrictEqualTyped({
-        data: { hello: "from link" },
+        data: { user: { __typename: "User", id: 1, name: "User 1" } },
         loading: false,
         networkStatus: NetworkStatus.ready,
         previousData: undefined,
@@ -7545,23 +7529,23 @@ describe("useQuery Hook", () => {
           reason: "variables-changed",
         })
       );
-      // the return value of `nextFetchPolicy(..., {reason: "variables-changed"})`
-      expectQueryTriggered(2, "cache-and-network");
 
+      // We now see the effects of cache-and-network applied
       await expect(takeSnapshot()).resolves.toStrictEqualTyped({
-        // TODO: Shouldn't this be undefined?
-        data: { hello: "from link" },
+        data: { user: { __typename: "User", id: 2, name: "Cached User 2" } },
         loading: true,
         networkStatus: NetworkStatus.setVariables,
-        previousData: { hello: "from link" },
+        previousData: { user: { __typename: "User", id: 1, name: "User 1" } },
         variables: { id: 2 },
       });
 
       await expect(takeSnapshot()).resolves.toStrictEqualTyped({
-        data: { hello: "from link2" },
+        data: { user: { __typename: "User", id: 2, name: "User 2" } },
         loading: false,
         networkStatus: NetworkStatus.ready,
-        previousData: { hello: "from link" },
+        previousData: {
+          user: { __typename: "User", id: 2, name: "Cached User 2" },
+        },
         variables: { id: 2 },
       });