Skip to content

Commit

Permalink
fix(core): make sure all options of a query are reactive (#7081)
Browse files Browse the repository at this point in the history
* fix: make sure all options of a query are reactive

currently, only observer based options are updated between renders (like staleTime), but query level ones are not (like retryDelay). This makes no sense, so trying to streamline here

The deleted test checked for multiple useQuery calls in the tree with different options, which I don't think happens a lot in practice.

As a side-effect, the queryFn is also updated, so it will see values from a closure. This shouldn't matter because everything used in the queryFn must be part of the queryKey

* test: solid adapter
  • Loading branch information
TkDodo authored Mar 12, 2024
1 parent 990758b commit 605b8e9
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 27 deletions.
8 changes: 4 additions & 4 deletions packages/query-core/src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export class Query<

this.#abortSignalConsumed = false
this.#defaultOptions = config.defaultOptions
this.#setOptions(config.options)
this.setOptions(config.options)
this.#observers = []
this.#cache = config.cache
this.queryKey = config.queryKey
Expand All @@ -182,7 +182,7 @@ export class Query<
return this.options.meta
}

#setOptions(
setOptions(
options?: QueryOptions<TQueryFnData, TError, TData, TQueryKey>,
): void {
this.options = { ...this.#defaultOptions, ...options }
Expand Down Expand Up @@ -342,15 +342,15 @@ export class Query<

// Update config if passed, otherwise the config from the last execution is used
if (options) {
this.#setOptions(options)
this.setOptions(options)
}

// Use the options from the first observer with a query function if no function is found.
// This can happen when the query is hydrated or created with setQueryData.
if (!this.options.queryFn) {
const observer = this.#observers.find((x) => x.options.queryFn)
if (observer) {
this.#setOptions(observer.options)
this.setOptions(observer.options)
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/query-core/src/queryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export class QueryObserver<
}

this.#updateQuery()
this.#currentQuery.setOptions(this.options)

if (!shallowEqualObjects(this.options, prevOptions)) {
this.#client.getQueryCache().notify({
Expand Down
2 changes: 2 additions & 0 deletions packages/query-core/src/tests/queryClient.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -941,11 +941,13 @@ describe('queryClient', () => {
await queryClient.fetchQuery({ queryKey: key2, queryFn: queryFn2 })
const observer1 = new QueryObserver(queryClient, {
queryKey: key1,
queryFn: queryFn1,
staleTime: Infinity,
initialData: 'initial',
})
const observer2 = new QueryObserver(queryClient, {
queryKey: key1,
queryFn: queryFn1,
staleTime: Infinity,
initialData: 'initial',
})
Expand Down
21 changes: 8 additions & 13 deletions packages/react-query/src/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2435,28 +2435,23 @@ describe('useQuery', () => {
rendered.getByText('Second Status: success')
})

it('should not override query configuration on render', async () => {
it('should update query options', async () => {
const key = queryKey()

const queryFn1 = async () => {
const queryFn = 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 })
useQuery({ queryKey: key, queryFn, retryDelay: 10 })
useQuery({ queryKey: key, queryFn, retryDelay: 20 })
return null
}

renderWithClient(queryClient, <Page />)

expect(queryCache.find({ queryKey: key })!.options.queryFn).toBe(queryFn1)
expect(queryCache.find({ queryKey: key })!.options.retryDelay).toBe(20)
})

it('should batch re-renders', async () => {
Expand Down Expand Up @@ -3685,10 +3680,10 @@ describe('useQuery', () => {
it('should reset failureCount on successful fetch', async () => {
const key = queryKey()

function Page() {
let counter = 0
let counter = 0

const query = useQuery<string, Error>({
function Page() {
const query = useQuery({
queryKey: key,
queryFn: async () => {
if (counter < 2) {
Expand Down
15 changes: 5 additions & 10 deletions packages/solid-query/src/__tests__/createQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1890,22 +1890,17 @@ describe('createQuery', () => {
rendered.getByText('Second Status: success')
})

it('should not override query configuration on render', async () => {
it('should update query options', async () => {
const key = queryKey()

const queryFn1 = async () => {
const queryFn = async () => {
await sleep(10)
return 'data1'
}

const queryFn2 = async () => {
await sleep(10)
return 'data2'
}

function Page() {
createQuery(() => ({ queryKey: key, queryFn: queryFn1 }))
createQuery(() => ({ queryKey: key, queryFn: queryFn2 }))
createQuery(() => ({ queryKey: key, queryFn, retryDelay: 10 }))
createQuery(() => ({ queryKey: key, queryFn, retryDelay: 20 }))
return null
}

Expand All @@ -1915,7 +1910,7 @@ describe('createQuery', () => {
</QueryClientProvider>
))

expect(queryCache.find({ queryKey: key })!.options.queryFn).toBe(queryFn1)
expect(queryCache.find({ queryKey: key })!.options.retryDelay).toBe(20)
})

it('should batch re-renders', async () => {
Expand Down

0 comments on commit 605b8e9

Please sign in to comment.