Hotfixes

CHANGELOG.md

@@ -1,3 +1,14 @@
+## Apollo Client 3.5.8 (unreleased)
+
+### Bug Fixes
+- Fix the type of the `called` property for `useQuery()`/`useLazyQuery()`. <br/>
+  [@sztadii](https://github.com/sztadii) in [#9304](https://github.com/apollographql/apollo-client/pull/9304)
+
+###  Bug Fixes (by [@brainkim](https://github.com/brainkim) in [#9328](https://github.com/apollographql/apollo-client/pull/9328))
+- Fix `refetch()` not being called when `skip` is true.
+- Fix the promise returned from the `useLazyQuery()` execution function having stale variables.
+- Fix the promise returned from the `useLazyQuery()` execution function not rejecting when a query errors.
+
 ## Apollo Client 3.5.7 (2022-01-10)
 
 ### Bug Fixes

src/core/ObservableQuery.ts

@@ -326,7 +326,7 @@ export class ObservableQuery<
     // (no-cache, network-only, or cache-and-network), override it with
     // network-only to force the refetch for this fetchQuery call.
     const { fetchPolicy } = this.options;
-    if (fetchPolicy === 'standby' || fetchPolicy === 'cache-and-network') {
+    if (fetchPolicy === 'cache-and-network') {
       reobserveOptions.fetchPolicy = fetchPolicy;
     } else if (fetchPolicy === 'no-cache') {
       reobserveOptions.fetchPolicy = 'no-cache';

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

@@ -1,4 +1,5 @@
 import React from 'react';
+import { GraphQLError } from 'graphql';
 import gql from 'graphql-tag';
 import { renderHook } from '@testing-library/react-hooks';
 
@@ -450,6 +451,7 @@ describe('useLazyQuery Hook', () => {
     expect(result.current[1].previousData).toBe(undefined);
 
     setTimeout(() => execute({ variables: { id: 2 }}));
+
     await waitForNextUpdate();
     expect(result.current[1].loading).toBe(true);
     expect(result.current[1].data).toBe(undefined);
@@ -530,18 +532,197 @@ describe('useLazyQuery Hook', () => {
     expect(result.current[1].loading).toBe(false);
     expect(result.current[1].data).toBe(undefined);
     const execute = result.current[0];
-    const mock = jest.fn();
-    setTimeout(() => mock(execute()));
+    let executeResult: any;
+    setTimeout(() => {
+      executeResult = execute();
+    });
 
     await waitForNextUpdate();
     expect(result.current[1].loading).toBe(true);
 
     await waitForNextUpdate();
     expect(result.current[1].loading).toBe(false);
     expect(result.current[1].data).toEqual({ hello: 'world' });
+    await expect(executeResult).resolves.toEqual(result.current[1]);
+  });
+
+  it('should have matching results from execution function and hook', async () => {
+    const query = gql`
+      query GetCountries($filter: String) {
+        countries(filter: $filter) {
+          code
+          name
+        }
+      }
+    `;
+
+    const mocks = [
+      {
+        request: {
+          query,
+          variables: {
+            filter: "PA",
+          },
+        },
+        result: {
+          data: {
+            countries: {
+              code: "PA",
+              name: "Panama",
+            },
+          },
+        },
+        delay: 20,
+      },
+      {
+        request: {
+          query,
+          variables: {
+            filter: "BA",
+          },
+        },
+        result: {
+          data: {
+            countries: {
+              code: "BA",
+              name: "Bahamas",
+            },
+          },
+        },
+        delay: 20,
+      },
+    ];
+
+    const { result, waitForNextUpdate } = renderHook(
+      () => useLazyQuery(query),
+      {
+        wrapper: ({ children }) => (
+          <MockedProvider mocks={mocks}>
+            {children}
+          </MockedProvider>
+        ),
+      },
+    );
+
+    expect(result.current[1].loading).toBe(false);
+    expect(result.current[1].data).toBe(undefined);
+    const execute = result.current[0];
+    let executeResult: any;
+    setTimeout(() => {
+      executeResult = execute({ variables: { filter: "PA" } });
+    });
+
+    await waitForNextUpdate();
+    expect(result.current[1].loading).toBe(true);
+
+    await waitForNextUpdate();
+    expect(result.current[1].loading).toBe(false);
+    expect(result.current[1].data).toEqual({
+      countries: {
+        code: "PA",
+        name: "Panama",
+      },
+    });
+
+    expect(executeResult).toBeInstanceOf(Promise);
+    expect((await executeResult).data).toEqual({
+      countries: {
+        code: "PA",
+        name: "Panama",
+      },
+    });
+
+    setTimeout(() => {
+      executeResult = execute({ variables: { filter: "BA" } });
+    });
+
+    await waitForNextUpdate();
+    // TODO: Get rid of this render.
+
+    await waitForNextUpdate();
+    expect(result.current[1].loading).toBe(false);
+    expect(result.current[1].data).toEqual({
+      countries: {
+        code: "BA",
+        name: "Bahamas",
+      },
+    });
+
+    expect(executeResult).toBeInstanceOf(Promise);
+    expect((await executeResult).data).toEqual({
+      countries: {
+        code: "BA",
+        name: "Bahamas",
+      },
+    });
+  });
+
+  it('the promise should reject with errors the “way useMutation does”', async () => {
+    const query = gql`{ hello }`;
+    const mocks = [
+      {
+        request: { query },
+        result: {
+          errors: [new GraphQLError('error 1')],
+        },
+        delay: 20,
+      },
+      {
+        request: { query },
+        result: {
+          errors: [new GraphQLError('error 2')],
+        },
+        delay: 20,
+      },
+    ];
+
+    const { result, waitForNextUpdate } = renderHook(
+      () => useLazyQuery(query),
+      {
+        wrapper: ({ children }) => (
+          <MockedProvider mocks={mocks}>
+            {children}
+          </MockedProvider>
+        ),
+      },
+    );
+
+    const execute = result.current[0];
+    let executeResult: any;
+    expect(result.current[1].loading).toBe(false);
+    expect(result.current[1].data).toBe(undefined);
+    setTimeout(() => {
+      executeResult = execute();
+      executeResult.catch(() => {});
+    });
+
+    await waitForNextUpdate();
+    expect(result.current[1].loading).toBe(true);
+    expect(result.current[1].data).toBe(undefined);
+    expect(result.current[1].error).toBe(undefined);
+
+    await waitForNextUpdate();
+    expect(result.current[1].loading).toBe(false);
+    expect(result.current[1].data).toBe(undefined);
+    expect(result.current[1].error).toEqual(new Error('error 1'));
+
+    await expect(executeResult).rejects.toEqual(new Error('error 1'));
+
+    setTimeout(() => {
+      executeResult = execute();
+      executeResult.catch(() => {});
+    });
+
+    await waitForNextUpdate();
+    expect(result.current[1].loading).toBe(false);
+    expect(result.current[1].data).toBe(undefined);
+    expect(result.current[1].error).toEqual(new Error('error 1'));
+
+    await waitForNextUpdate();
+    expect(result.current[1].loading).toBe(false);
+    expect(result.current[1].data).toBe(undefined);
+    expect(result.current[1].error).toEqual(new Error('error 2'));
 
-    expect(mock).toHaveBeenCalledTimes(1);
-    expect(mock.mock.calls[0][0]).toBeInstanceOf(Promise);
-    expect(await mock.mock.calls[0][0]).toEqual(result.current[1]);
+    await expect(executeResult).rejects.toEqual(new Error('error 2'));
   });
 });

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

@@ -272,6 +272,49 @@ describe('useMutation Hook', () => {
         expect(onError.mock.calls[0][0].message).toBe(CREATE_TODO_ERROR);
       });
 
+      it('should reject when there’s only an error and no error policy is set', async () => {
+        const variables = {
+          description: 'Get milk!'
+        };
+
+        const mocks = [
+          {
+            request: {
+              query: CREATE_TODO_MUTATION,
+              variables,
+            },
+            result: {
+              errors: [new GraphQLError(CREATE_TODO_ERROR)],
+            },
+          }
+        ];
+
+        const { result } = renderHook(
+          () => useMutation(CREATE_TODO_MUTATION),
+          { wrapper: ({ children }) => (
+            <MockedProvider mocks={mocks}>
+              {children}
+            </MockedProvider>
+          )},
+        );
+
+        const createTodo = result.current[0];
+        let fetchError: any;
+        await act(async () => {
+          // need to call createTodo this way to get “act” warnings to go away.
+          try {
+            await createTodo({ variables });
+          } catch (err) {
+            fetchError = err;
+            return;
+          }
+
+          throw new Error("function did not error");
+        });
+
+        expect(fetchError).toEqual(new GraphQLError(CREATE_TODO_ERROR));
+      });
+
       it(`should reject when errorPolicy is 'none'`, async () => {
         const variables = {
           description: 'Get milk!'
@@ -341,7 +384,47 @@ describe('useMutation Hook', () => {
 
         expect(fetchResult.data).toEqual(CREATE_TODO_RESULT);
         expect(fetchResult.errors[0].message).toEqual(CREATE_TODO_ERROR);
-      })
+      });
+
+      it(`should ignore errors when errorPolicy is 'ignore'`, async () => {
+        const errorMock = jest.spyOn(console, "error")
+          .mockImplementation(() => {});
+        const variables = {
+          description: 'Get milk!'
+        };
+
+        const mocks = [
+          {
+            request: {
+              query: CREATE_TODO_MUTATION,
+              variables,
+            },
+            result: {
+              errors: [new GraphQLError(CREATE_TODO_ERROR)],
+            },
+          }
+        ];
+
+        const { result } = renderHook(
+          () => useMutation(CREATE_TODO_MUTATION, { errorPolicy: "ignore" }),
+          { wrapper: ({ children }) => (
+            <MockedProvider mocks={mocks}>
+              {children}
+            </MockedProvider>
+          )},
+        );
+
+        const createTodo = result.current[0];
+        let fetchResult: any;
+        await act(async () => {
+          fetchResult = await createTodo({ variables });
+        });
+
+        expect(fetchResult).toEqual({});
+        expect(errorMock).toHaveBeenCalledTimes(1);
+        expect(errorMock.mock.calls[0][0]).toMatch("Missing field");
+        errorMock.mockRestore();
+      });
     });
 
     it('should return the current client instance in the result object', async () => {

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

@@ -3071,7 +3071,8 @@ describe('useQuery Hook', () => {
       expect(result.current.data).toEqual({ hello: 'world' });
     });
 
-    it('should not refetch when skip is true', async () => {
+    // Amusingly, #8270 thinks this is a bug, but #9101 thinks this is not.
+    it('should refetch when skip is true', async () => {
       const query = gql`{ hello }`;
       const link = new ApolloLink(() => Observable.of({
         data: { hello: 'world' },
@@ -3098,13 +3099,18 @@ describe('useQuery Hook', () => {
       expect(result.current.data).toBe(undefined);
       await expect(waitForNextUpdate({ timeout: 20 }))
         .rejects.toThrow('Timed out');
-      result.current.refetch();
-      await expect(waitForNextUpdate({ timeout: 20 }))
-        .rejects.toThrow('Timed out');
+      const promise = result.current.refetch();
+      // TODO: Not really sure about who is causing this render.
+      await waitForNextUpdate();
       expect(result.current.loading).toBe(false);
       expect(result.current.data).toBe(undefined);
-      expect(requestSpy).toHaveBeenCalledTimes(0);
+      expect(requestSpy).toHaveBeenCalledTimes(1);
       requestSpy.mockRestore();
+      expect(promise).resolves.toEqual({
+        data: {hello: "world"},
+        loading: false,
+        networkStatus: 7,
+      });
     });
   });