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: set options for a query on setOptions in a queryObserver #4749

Closed

Conversation

artem-malko
Copy link
Contributor

This PR attempts to close #3706

Opened questions

  • How to write a test for my fix? It looks quite complex, so I need an e2e test?

@TkDodo fyi, cause you have the context of the issue)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 3, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8c2d5b3:

Sandbox Source
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-vue-basic Configuration
react-query fetchQuery + useQuery Issue #3706

@artem-malko artem-malko changed the title Set options for a query on setOptions in a queryObserver fix: set options for a query on setOptions in a queryObserver Jan 3, 2023
@TkDodo
Copy link
Collaborator

TkDodo commented Jan 3, 2023

How to write a test for my fix? It looks quite complex, so I need an e2e test?

All our tests are "e2e" tests because they mount a query on a page and then have buttons to interact with them. I think you could just write what your sandbox has and assert accordingly.

However, what bothers me is that it seems like we have a test that tests exactly the opposite of what we're trying to achieve, and that's obviously failing now:

it('should not override query configuration on render', async () => {
const key = queryKey()
const queryFn1 = async () => {
await sleep(10)
return 'data1'
}
const queryFn2 = async () => {
await sleep(10)
return 'data2'
}
function Page() {
useQuery({ queryKey: key, queryFn: queryFn1 })
useQuery({ queryKey: key, queryFn: queryFn2 })
return null
}
renderWithClient(queryClient, <Page />)
expect(queryCache.find({ queryKey: key })!.options.queryFn).toBe(queryFn1)
})

@TkDodo
Copy link
Collaborator

TkDodo commented Jan 3, 2023

fyi, the test was introduced here:

@artem-malko
Copy link
Contributor Author

Hm, I'm quite confused)

First of all, that tests checks only queryFunc, not options. So, as I understand, we can save queryFunc between executions, but all other options can be updated?

@TkDodo
Copy link
Collaborator

TkDodo commented Jan 5, 2023

First of all, that tests checks only queryFunc, not options. So, as I understand, we can save queryFunc between executions, but all other options can be updated?

I think the test only checks for queryFn because queryFn is one of the valid options that live on the query itself. It could just as well check any other option.

The PR description says:

Currently the query configuration is dependent on the order in which hooks are rendered and this could result in unexpected behaviour. This MR will make sure the query configuration is only changed on execution. Related to #932

@artem-malko
Copy link
Contributor Author

I'm still confused a little( Looks like current behaviour (which is a bug for me) is a correct from react-query side =(

@artem-malko
Copy link
Contributor Author

I'll close this request. Looks like it is quite outdated and it is not so useful for the community) I'll watch for that issue by using v5 and I'll be back with a feedback)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fetchQuery and useQuery options
2 participants