diff --git a/CHANGELOG.md b/CHANGELOG.md
index 65015013265..b58206788cd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## Apollo Client 3.8.0
+
+### Bug fixes
+
+- Avoid calling `useQuery` `onCompleted` callback after cache writes, only after the originating query's network request(s) complete.
+ [@alessbell](https://github.com/alessbell) in [#10229](https://github.com/apollographql/apollo-client/pull/10229)
+
## Apollo Client 3.7.1 (2022-10-20)
### Bug fixes
diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
index 1b023b058ec..c0a53235473 100644
--- a/src/react/hooks/__tests__/useQuery.test.tsx
+++ b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -3,7 +3,8 @@ import React, { Fragment, useEffect, useState } from 'react';
import { DocumentNode, GraphQLError } from 'graphql';
import gql from 'graphql-tag';
import { act } from 'react-dom/test-utils';
-import { render, waitFor } from '@testing-library/react';
+import { render, screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
import { renderHook } from '@testing-library/react-hooks';
import {
ApolloClient,
@@ -3069,7 +3070,7 @@ describe('useQuery Hook', () => {
expect(onCompleted).toHaveBeenCalledTimes(1);
});
- it('onCompleted should work with polling', async () => {
+ it('onCompleted should not fire for polling queries without notifyOnNetworkStatusChange: true', async () => {
const query = gql`{ hello }`;
const mocks = [
{
@@ -3112,6 +3113,68 @@ describe('useQuery Hook', () => {
await waitForNextUpdate();
expect(result.current.loading).toBe(false);
expect(result.current.data).toEqual({ hello: 'world 2' });
+ expect(onCompleted).toHaveBeenCalledTimes(1);
+
+ await waitForNextUpdate();
+ expect(result.current.loading).toBe(false);
+ expect(result.current.data).toEqual({ hello: 'world 3' });
+ expect(onCompleted).toHaveBeenCalledTimes(1);
+ });
+
+ it('onCompleted should fire when polling with notifyOnNetworkStatusChange: true', async () => {
+ const query = gql`{ hello }`;
+ const mocks = [
+ {
+ request: { query },
+ result: { data: { hello: 'world 1' } },
+ },
+ {
+ request: { query },
+ result: { data: { hello: 'world 2' } },
+ },
+ {
+ request: { query },
+ result: { data: { hello: 'world 3' } },
+ },
+ ];
+
+ const cache = new InMemoryCache();
+ const onCompleted = jest.fn();
+ const { result, waitForNextUpdate } = renderHook(
+ () => useQuery(query, {
+ onCompleted,
+ notifyOnNetworkStatusChange: true,
+ pollInterval: 10,
+ }),
+ {
+ wrapper: ({ children }) => (
+
+ {children}
+
+ ),
+ },
+ );
+
+ expect(result.current.loading).toBe(true);
+
+ await waitForNextUpdate();
+ expect(result.current.loading).toBe(false);
+ expect(result.current.data).toEqual({ hello: 'world 1' });
+ expect(onCompleted).toHaveBeenCalledTimes(1);
+
+ await waitForNextUpdate();
+ expect(result.current.loading).toBe(true);
+ expect(result.current.data).toEqual({ hello: 'world 1' });
+ expect(onCompleted).toHaveBeenCalledTimes(1);
+
+ await waitForNextUpdate();
+ expect(result.current.loading).toBe(false);
+ expect(result.current.data).toEqual({ hello: 'world 2' });
+ expect(onCompleted).toHaveBeenCalledTimes(2);
+
+ await waitForNextUpdate();
+ expect(result.current.loading).toBe(true);
+ expect(result.current.data).toEqual({ hello: 'world 2' });
expect(onCompleted).toHaveBeenCalledTimes(2);
await waitForNextUpdate();
@@ -3165,6 +3228,133 @@ describe('useQuery Hook', () => {
expect(errorSpy).not.toHaveBeenCalled();
errorSpy.mockRestore();
});
+
+ it("onCompleted should not execute on cache writes after initial query execution", async () => {
+ const query = gql`
+ {
+ hello
+ }
+ `;
+ const mocks = [
+ {
+ request: { query },
+ result: { data: { hello: "foo" } },
+ },
+ {
+ request: { query },
+ result: { data: { hello: "bar" } },
+ },
+ ];
+ const link = new MockLink(mocks);
+ const cache = new InMemoryCache();
+ const onCompleted = jest.fn();
+
+ const ChildComponent: React.FC = () => {
+ const { data, client } = useQuery(query, { onCompleted });
+ function refetchQueries() {
+ client.refetchQueries({ include: 'active' })
+ }
+ function writeQuery() {
+ client.writeQuery({ query, data: { hello: 'baz'}})
+ }
+ return (
+
+ Data: {data?.hello}
+
+
+
+ );
+ };
+
+ const ParentComponent: React.FC = () => {
+ return (
+
+
+
+
+
+ );
+ };
+
+ render();
+
+ await screen.findByText("Data: foo");
+ await userEvent.click(screen.getByRole('button', { name: /refetch queries/i }));
+ expect(onCompleted).toBeCalledTimes(1);
+ await screen.findByText("Data: bar");
+ await userEvent.click(screen.getByRole('button', { name: /update word/i }));
+ expect(onCompleted).toBeCalledTimes(1);
+ await screen.findByText("Data: baz");
+ expect(onCompleted).toBeCalledTimes(1);
+ });
+
+ it("onCompleted should execute on cache writes after initial query execution with notifyOnNetworkStatusChange: true", async () => {
+ const query = gql`
+ {
+ hello
+ }
+ `;
+ const mocks = [
+ {
+ request: { query },
+ result: { data: { hello: "foo" } },
+ },
+ {
+ request: { query },
+ result: { data: { hello: "bar" } },
+ },
+ ];
+ const link = new MockLink(mocks);
+ const cache = new InMemoryCache();
+ const onCompleted = jest.fn();
+
+ const ChildComponent: React.FC = () => {
+ const { data, client } = useQuery(
+ query,
+ {
+ onCompleted,
+ notifyOnNetworkStatusChange: true
+ }
+ );
+ function refetchQueries() {
+ client.refetchQueries({ include: 'active' })
+ }
+ function writeQuery() {
+ client.writeQuery({ query, data: { hello: 'baz'}})
+ }
+ return (
+
+ Data: {data?.hello}
+
+
+
+ );
+ };
+
+ const ParentComponent: React.FC = () => {
+ return (
+
+
+
+
+
+ );
+ };
+
+ render();
+
+ await screen.findByText("Data: foo");
+ expect(onCompleted).toBeCalledTimes(1);
+ await userEvent.click(screen.getByRole('button', { name: /refetch queries/i }));
+ // onCompleted increments when refetch occurs since we're hitting the network...
+ expect(onCompleted).toBeCalledTimes(2);
+ await screen.findByText("Data: bar");
+ await userEvent.click(screen.getByRole('button', { name: /update word/i }));
+ // but not on direct cache write, since there's no network request to complete
+ expect(onCompleted).toBeCalledTimes(2);
+ await screen.findByText("Data: baz");
+ expect(onCompleted).toBeCalledTimes(2);
+ });
});
describe('Optimistic data', () => {
diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts
index 2bf72a83329..0758499c380 100644
--- a/src/react/hooks/useQuery.ts
+++ b/src/react/hooks/useQuery.ts
@@ -498,16 +498,23 @@ class InternalState {
// Calling state.setResult always triggers an update, though some call sites
// perform additional equality checks before committing to an update.
this.forceUpdate();
- this.handleErrorOrCompleted(nextResult);
+ this.handleErrorOrCompleted(nextResult, previousResult);
}
- private handleErrorOrCompleted(result: ApolloQueryResult) {
+ private handleErrorOrCompleted(
+ result: ApolloQueryResult,
+ previousResult?: ApolloQueryResult
+ ) {
if (!result.loading) {
// wait a tick in case we are in the middle of rendering a component
Promise.resolve().then(() => {
if (result.error) {
this.onError(result.error);
- } else if (result.data) {
+ } else if (
+ result.data &&
+ previousResult?.networkStatus !== result.networkStatus &&
+ result.networkStatus === NetworkStatus.ready
+ ) {
this.onCompleted(result.data);
}
}).catch(error => {