Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: useQuery returns called value based on skip option. #9798

Merged
merged 2 commits into from
Jun 10, 2022

Conversation

KucharskiPiotr
Copy link
Contributor

Changed returned called value of QueryResult to be false, when
the skip parameter of QueryHookOptions is set to true. Otherwise,
called value should return true like it used to do by default.

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

Reason to change:

useQuery result documentation states that called value should be true only if the query has been executed. Currently, when useQuery is called with option { skip: true }, the value of called remains true, even though the network request hasn't been done. This was discussed in #7931.

This Pull Request changes value of called to be based on the skip option - if the option was set to true, the called value will be set to false, which should work as expected.

@apollo-cla
Copy link

@KucharskiPiotr: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

Comment on lines +221 to +242
it('should set called to false when skip option is true', async () => {
const query = gql`{ hello }`;
const mocks = [
{
request: { query },
result: { data: { hello: "world" } },
},
];

const cache = new InMemoryCache();
const wrapper = ({ children }: any) => (
<MockedProvider mocks={mocks} cache={cache}>{children}</MockedProvider>
);

const { result, unmount } = renderHook(
() => useQuery(query, { skip: true }),
{ wrapper },
);

expect(result.current.called).toBe(false);
unmount();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following test is copy-paste of the test above (should set called to true by default) with added { skip: true } parameter to useQuery and changed expected value to be false.

Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, and thanks for adding tests @KucharskiPiotr!

Changed returned `called` value of `QueryResult` to be false, when
the `skip` parameter of `QueryHookOptions` is set to true. Otherwise,
`called` value should return true like it used to do by default.
@benjamn benjamn removed the 🏓 awaiting-team-response requires input from the apollo team label Jun 10, 2022
@benjamn benjamn added this to the v3.6.x patch releases milestone Jun 10, 2022
@benjamn benjamn merged commit 8ec45a9 into apollographql:main Jun 10, 2022
@benjamn
Copy link
Member

benjamn commented Jun 10, 2022

@KucharskiPiotr These changes have been soft-published in @apollo/client@3.6.8, meaning you can install that version directly to test the changes, and we will promote it to latest probably this coming Monday, if everything looks good. Thanks again!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants