From 2aa3183127a4bd7187c687a3f6ee1c12ffb56c58 Mon Sep 17 00:00:00 2001
From: hwillson <hugh@octonary.com>
Date: Mon, 13 Jul 2020 14:58:12 -0400
Subject: [PATCH] Ignore option callbacks when deciding to update a query
`QueryData` stores last used options to help decide when it should
re-run. If new options (when compared against the previously
stored last options) are found, `QueryData` will make sure the
new options are passed into Apollo Client for processing.
When `onCompleted` and/or `onError` options are set however,
`QueryData` thinks the options received on each render are new
as these callback functions don't have a stable identity. This
can then lead to infinite re-renders.
This commit adjusts the `QueryData` option equality check to
ignore option callbacks. During normal use of `useQuery` it
should be okay to ignore callbacks like this, as they don't
normally change between renders.
Fixes #6301
---
src/react/data/QueryData.ts | 27 +++++++++++----
src/react/hooks/__tests__/useQuery.test.tsx | 37 +++++++++++++++++++++
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/src/react/data/QueryData.ts b/src/react/data/QueryData.ts
index 4d3ad689830..4f041a1399a 100644
--- a/src/react/data/QueryData.ts
+++ b/src/react/data/QueryData.ts
@@ -239,12 +239,7 @@ export class QueryData<TData, TVariables> extends OperationData {
children: null
};
- if (
- !equal(
- newObservableQueryOptions,
- this.previousData.observableQueryOptions
- )
- ) {
+ if (this.haveOptionsChanged(newObservableQueryOptions)) {
this.previousData.observableQueryOptions = newObservableQueryOptions;
this.currentObservable
.setOptions(newObservableQueryOptions)
@@ -484,4 +479,24 @@ export class QueryData<TData, TVariables> extends OperationData {
subscribeToMore: this.obsSubscribeToMore
} as ObservableQueryFields<TData, TVariables>;
}
+
+ private haveOptionsChanged(options: QueryDataOptions<TData, TVariables>) {
+ // When comparing new options against previously stored options,
+ // we'll ignore callback functions since their identities are not
+ // stable, meaning they'll always show as being different. Ignoring
+ // them when determining if options have changed is okay however, as
+ // callback functions are not normally changed between renders.
+ return !equal(
+ {
+ ...options,
+ onCompleted: undefined,
+ onError: undefined
+ },
+ {
+ ...this.previousData.observableQueryOptions,
+ onCompleted: undefined,
+ onError: undefined
+ }
+ );
+ }
}
diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
index 9ec841088e5..1787d437494 100644
--- a/src/react/hooks/__tests__/useQuery.test.tsx
+++ b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -1798,6 +1798,43 @@ describe('useQuery Hook', () => {
expect(renderCount).toBe(3);
}).then(resolve, reject);
});
+
+ itAsync(
+ 'should not make extra network requests when `onCompleted` is ' +
+ 'defined with a `network-only` fetch policy',
+ (resolve, reject) => {
+ let renderCount = 0;
+ function Component() {
+ const { loading, data } = useQuery(CAR_QUERY, {
+ fetchPolicy: 'network-only',
+ onCompleted: () => undefined
+ });
+ switch (++renderCount) {
+ case 1:
+ expect(loading).toBeTruthy();
+ break;
+ case 2:
+ expect(loading).toBeFalsy();
+ expect(data).toEqual(CAR_RESULT_DATA);
+ break;
+ case 3:
+ fail('Too many renders');
+ default:
+ }
+ return null;
+ }
+
+ render(
+ <MockedProvider mocks={CAR_MOCKS}>
+ <Component />
+ </MockedProvider>
+ );
+
+ return wait(() => {
+ expect(renderCount).toBe(2);
+ }).then(resolve, reject);
+ }
+ );
});
describe('Optimistic data', () => {