Skip to content

Commit

Permalink
Ignore option callbacks when deciding to update a query
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
hwillson committed Jul 13, 2020
1 parent 287aa46 commit 2aa3183
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 6 deletions.
27 changes: 21 additions & 6 deletions src/react/data/QueryData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
);
}
}
37 changes: 37 additions & 0 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 2aa3183

Please sign in to comment.