Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions docs/framework/react/reference/useQuery.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const {
notifyOnChangeProps,
placeholderData,
queryKeyHashFn,
onSuccess,
onError,
onSettled,
refetchInterval,
refetchIntervalInBackground,
refetchOnMount,
Expand Down Expand Up @@ -106,6 +109,18 @@ const {
- `queryKeyHashFn: (queryKey: QueryKey) => string`
- Optional
- If specified, this function is used to hash the `queryKey` to a string.
- `onSuccess: (data: TData | undefined) => Promise<unknown> | unknown`
- Optional
- This function will fire when the `queryFn` is successful and will be passed the `data`.
- Void function, the returned value will be ignored
- `onError: (error: TError | null) => Promise<unknown> | unknown`
- Optional
- This function will fire if the `queryFn` encounters an error and will be passed the `error`
- Void function, the returned value will be ignored
- `onSettled: (data: TData | undefined, error: TError | null) => Promise<unknown> | unknown`
- Optional
- This function will fire when the `queryFn` is either successfully fetched or encounters an error and be passed either the `data` or `error`
- Void function, the returned value will be ignored
- `refetchInterval: number | false | ((query: Query) => number | false | undefined)`
- Optional
- If set to a number, all queries will continuously refetch at this frequency in milliseconds
Expand Down
15 changes: 15 additions & 0 deletions docs/framework/solid/reference/useQuery.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const {
initialDataUpdatedAt,
meta,
queryKeyHashFn,
onSuccess,
onError,
onSettled,
refetchInterval,
refetchIntervalInBackground,
refetchOnMount,
Expand Down Expand Up @@ -244,6 +247,18 @@ function App() {
- ##### `queryKeyHashFn: (queryKey: QueryKey) => string`
- Optional
- If specified, this function is used to hash the `queryKey` to a string.
- ##### `onSuccess: (data: TData | undefined) => Promise<unknown> | unknown`
- Optional
- This function will fire when the `queryFn` is successful and will be passed the `data`.
- Void function, the returned value will be ignored
- ##### `onError: (error: TError | null) => Promise<unknown> | unknown`
- Optional
- This function will fire if the `queryFn` encounters an error and will be passed the `error`
- Void function, the returned value will be ignored
- ##### `onSettled: (data: TData | undefined, error: TError | null) => Promise<unknown> | unknown`
- Optional
- This function will fire when the `queryFn` is either successfully fetched or encounters an error and be passed either the `data` or `error`
- Void function, the returned value will be ignored
- ##### `refetchInterval: number | false | ((query: Query) => number | false | undefined)`
- Optional
- If set to a number, all queries will continuously refetch at this frequency in milliseconds
Expand Down
12 changes: 9 additions & 3 deletions examples/react/nextjs-suspense-streaming/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
{
"compilerOptions": {
"target": "ES5",
"lib": ["dom", "dom.iterable", "esnext"],
"lib": [
"dom",
"dom.iterable",
"esnext"
],
"allowJs": true,
"skipLibCheck": true,
"strict": true,
Expand All @@ -12,7 +16,7 @@
"moduleResolution": "Bundler",
"resolveJsonModule": true,
"isolatedModules": true,
"jsx": "preserve",
"jsx": "react-jsx",
"incremental": true,
"plugins": [
{
Expand All @@ -27,5 +31,7 @@
".next/types/**/*.ts",
".next/dev/types/**/*.ts"
],
"exclude": ["node_modules"]
"exclude": [
"node_modules"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -766,4 +766,37 @@ describe('injectQuery', () => {
expect(callCount).toBe(2)
})
})


test('callbacks `onSuccess`, `onError` and `onSettled` should be called after a successful fetch', async () => {
const onSuccessMock = vi.fn()
const onFailureMock = vi.fn()
const onSuccessSettledMock = vi.fn()
const onFailureSettledMock = vi.fn()


TestBed.runInInjectionContext(() => {
injectQuery(() => ({
queryKey: ['expected-success'],
queryFn: () => 'fetched',
onSuccess: (data) => onSuccessMock(data),
onSettled: (data, _) => onSuccessSettledMock(data),
}));

injectQuery(() => ({
queryKey: ['expected-failure'],
queryFn: () => Promise.reject(new Error('error')),
onError: (error) => onFailureMock(error),
onSettled: (_, error) => onFailureSettledMock(error),
retry: false,
}));
})

await vi.advanceTimersByTimeAsync(10)

expect(onSuccessMock).toHaveBeenCalled()
expect(onSuccessSettledMock).toHaveBeenCalled()
expect(onFailureMock).toHaveBeenCalled()
expect(onFailureSettledMock).toHaveBeenCalled()
})
})
53 changes: 53 additions & 0 deletions packages/query-core/src/__tests__/query.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,29 @@ describe('query', () => {
})
})

test('query options callbacks should be called in correct order with correct arguments for error case', async () => {
const onError = vi.fn()
const onSettled = vi.fn()

const key = queryKey()

await expect(
queryClient.fetchQuery({
queryKey: key,
queryFn: () => Promise.reject(new Error('error')),
retry: false,
onError,
onSettled,
}),
).rejects.toBeDefined()

expect(onError).toHaveBeenCalledTimes(1)
expect(onError.mock.calls[0]![0]).toEqual(expect.any(Error))

expect(onSettled).toHaveBeenCalledTimes(1)
expect(onSettled).toHaveBeenCalledWith(undefined, expect.any(Error))
})

test('should not increment dataUpdateCount when setting initialData on prefetched query', async () => {
const key = queryKey()
const queryFn = vi.fn().mockImplementation(() => 'fetched-data')
Expand Down Expand Up @@ -1347,4 +1370,34 @@ describe('query', () => {
const updatedResult = observer.getCurrentResult()
expect(updatedResult.isFetchedAfterMount).toBe(true)
})


test('callbacks `onSuccess`, `onError` and `onSettled` should be called', async () => {
const onSuccessMock = vi.fn()
const onFailureMock = vi.fn()
const onSuccessSettledMock = vi.fn()
const onFailureSettledMock = vi.fn()

await queryClient.fetchQuery({
queryKey: ['expected-success'],
queryFn: () => 'fetched',
onSuccess: onSuccessMock,
onSettled: onSuccessSettledMock,
})

await expect(
queryClient.fetchQuery({
queryKey: ['expected-failure'],
queryFn: () => Promise.reject(new Error('error')),
onError: (error) => onFailureMock(error),
onSettled: (_, error) => onFailureSettledMock(error),
retry: false
})
).rejects.toBeDefined()

expect(onSuccessMock).toHaveBeenCalled()
expect(onSuccessSettledMock).toHaveBeenCalled()
expect(onFailureMock).toHaveBeenCalled()
expect(onFailureSettledMock).toHaveBeenCalled()
})
})
56 changes: 56 additions & 0 deletions packages/query-core/src/__tests__/queryObserver.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,62 @@ describe('queryObserver', () => {
expect(observer.getCurrentResult().data).toBe(selectedData2)
})

test('observer callbacks should be called in correct order with correct arguments for success case', async () => {
const onSuccess = vi.fn()
const onSettled = vi.fn()

const key = queryKey()

const observer = new QueryObserver(queryClient, {
queryKey: key,
queryFn: () => Promise.resolve('SUCCESS'),
onSuccess,
onSettled,
})

const unsubscribe = observer.subscribe(() => undefined)

await vi.advanceTimersByTimeAsync(0)

expect(onSuccess).toHaveBeenCalled()
expect(onSuccess).toHaveBeenLastCalledWith('SUCCESS')

expect(onSettled).toHaveBeenCalled()
expect(onSettled).toHaveBeenLastCalledWith('SUCCESS', null)

unsubscribe()
})

test('observer callbacks should be called in correct order with correct arguments for error case', async () => {
const onError = vi.fn()
const onSettled = vi.fn()

const err = new Error('err')
const key = queryKey()

const observer = new QueryObserver(queryClient, {
queryKey: key,
queryFn: () => Promise.reject(err),
retry: false,
onError,
onSettled,
})

const unsubscribe = observer.subscribe(() => undefined)

await vi.advanceTimersByTimeAsync(0)

expect(onError).toHaveBeenCalled()
const onErrorLast = onError.mock.calls.at(-1)![0]
expect(onErrorLast).toEqual(expect.any(Error))
expect(onErrorLast.message).toBe('err')

expect(onSettled).toHaveBeenCalled()
expect(onSettled).toHaveBeenLastCalledWith(undefined, expect.any(Error))

unsubscribe()
})

test('should pass the correct previous queryKey (from prevQuery) to placeholderData function params with select', async () => {
const results: Array<QueryObserverResult> = []
const keys: Array<ReadonlyArray<unknown> | null> = []
Expand Down
14 changes: 14 additions & 0 deletions packages/query-core/src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,13 @@ export class Query<

this.setData(data)

// Notify per-query callback
await this.options.onSuccess?.(data)
await this.options.onSettled?.(
data,
this.state.error as any,
)

// Notify cache callback
this.#cache.config.onSuccess?.(data, this as Query<any, any, any, any>)
this.#cache.config.onSettled?.(
Expand Down Expand Up @@ -586,6 +593,13 @@ export class Query<
error: error as TError,
})

// Notify per-query callback
await this.options.onError?.(error as any)
await this.options.onSettled?.(
this.state.data,
error as any,
)

// Notify cache callback
this.#cache.config.onError?.(
error as any,
Expand Down
50 changes: 36 additions & 14 deletions packages/query-core/src/queryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export class QueryObserver<
typeof this.options.enabled !== 'boolean' &&
typeof this.options.enabled !== 'function' &&
typeof resolveEnabled(this.options.enabled, this.#currentQuery) !==
'boolean'
'boolean'
) {
throw new Error(
'Expected enabled to be a boolean or a callback that returns a boolean',
Expand Down Expand Up @@ -199,9 +199,9 @@ export class QueryObserver<
mounted &&
(this.#currentQuery !== prevQuery ||
resolveEnabled(this.options.enabled, this.#currentQuery) !==
resolveEnabled(prevOptions.enabled, this.#currentQuery) ||
resolveEnabled(prevOptions.enabled, this.#currentQuery) ||
resolveStaleTime(this.options.staleTime, this.#currentQuery) !==
resolveStaleTime(prevOptions.staleTime, this.#currentQuery))
resolveStaleTime(prevOptions.staleTime, this.#currentQuery))
) {
this.#updateStaleTimeout()
}
Expand All @@ -213,7 +213,7 @@ export class QueryObserver<
mounted &&
(this.#currentQuery !== prevQuery ||
resolveEnabled(this.options.enabled, this.#currentQuery) !==
resolveEnabled(prevOptions.enabled, this.#currentQuery) ||
resolveEnabled(prevOptions.enabled, this.#currentQuery) ||
nextRefetchInterval !== this.#currentRefetchInterval)
) {
this.#updateRefetchInterval(nextRefetchInterval)
Expand Down Expand Up @@ -502,11 +502,11 @@ export class QueryObserver<
placeholderData =
typeof options.placeholderData === 'function'
? (
options.placeholderData as unknown as PlaceholderDataFunction<TQueryData>
)(
this.#lastQueryWithDefinedData?.state.data,
this.#lastQueryWithDefinedData as any,
)
options.placeholderData as unknown as PlaceholderDataFunction<TQueryData>
)(
this.#lastQueryWithDefinedData?.state.data,
this.#lastQueryWithDefinedData as any,
)
: options.placeholderData
}

Expand Down Expand Up @@ -606,7 +606,7 @@ export class QueryObserver<
const recreateThenable = () => {
const pending =
(this.#currentThenable =
nextResult.promise =
nextResult.promise =
pendingThenable())

finalizeThenableIfPossible(pending)
Expand Down Expand Up @@ -662,7 +662,7 @@ export class QueryObserver<
return
}

this.#currentResult = nextResult
this.#currentResult = nextResult
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Lifecycle callbacks should be based on observer result, not raw query.state

Right now #notify decides which callbacks to fire and which arguments to pass by inspecting notifyOptions.query.state.status/data/error. This diverges from the observer’s computed result in several important cases:

  • select (and select errors):

    • createResult can set status = 'error' and error = this.#selectError solely at the observer level.
    • query.state.status and query.state.error remain whatever the fetch layer set ('success' / null).
    • With the current logic, onError/error onSettled will never see select errors; instead, the success branch may fire with stale data.
  • placeholderData:

    • When placeholder data is injected, createResult marks status = 'success' and isPlaceholderData = true, but query.state.status can still be 'pending'.
    • The callbacks will not see this “successful” placeholder state at all.
  • Refetching after success/error:

    • For background refetches of a successful query, query.state.status stays 'success' while only fetchStatus toggles.
    • This means onSuccess / onSettled will fire both when refetch starts (no new data) and again when it completes, which is likely more often than intended.

Because updateResult already computes nextResult and assigns this.#currentResult immediately before calling #notify, you can avoid these inconsistencies by basing the callbacks on this.#currentResult (or nextResult) instead:

  • Use this.#currentResult.status to choose between success/error branches.
  • Pass this.#currentResult.data and this.#currentResult.error into onSuccess / onError / onSettled.
  • Optionally gate firing on meaningful transitions (e.g. status or dataUpdatedAt change) to avoid duplicate callbacks when only fetchStatus flips.

This would align the lifecycle hooks with what hook consumers actually see in UseQueryResult, and ensure select/placeholder behaviors are reflected correctly.

Also applies to: 731-752

🤖 Prompt for AI Agents
In packages/query-core/src/queryObserver.ts around lines 665 (also applies to
731-752), the notify logic currently reads notifyOptions.query.state.* to decide
which lifecycle callbacks to call and what args to pass; change it to use the
observer’s computed result (this.#currentResult or nextResult) instead: switch
on this.#currentResult.status for success/error branches, pass
this.#currentResult.data and this.#currentResult.error into
onSuccess/onError/onSettled, and add simple guards to only fire callbacks on
meaningful transitions (e.g., status change or dataUpdatedAt change) to avoid
duplicate calls when only fetchStatus flips.


const shouldNotifyListeners = (): boolean => {
if (!prevResult) {
Expand Down Expand Up @@ -698,7 +698,7 @@ export class QueryObserver<
})
}

this.#notify({ listeners: shouldNotifyListeners() })
this.#notify({ query: this.#currentQuery, listeners: shouldNotifyListeners() })
}

#updateQuery(): void {
Expand Down Expand Up @@ -728,9 +728,31 @@ export class QueryObserver<
}
}

#notify(notifyOptions: { listeners: boolean }): void {
#notify(notifyOptions: { query: Query<TQueryFnData, TError, TQueryData, TQueryKey>, listeners: boolean }): void {
notifyManager.batch(() => {
// First, trigger the listeners
// First trigger the mutate callbacks
if (this.hasListeners()) {

if (notifyOptions.query.state.status === 'success') {
this.options.onSuccess?.(
notifyOptions.query.state.data
)
this.options.onSettled?.(
notifyOptions.query.state.data,
null,
)
} else if (notifyOptions.query.state.status === 'error') {
this.options.onError?.(
notifyOptions.query.state.error,
)
this.options.onSettled?.(
undefined,
notifyOptions.query.state.error,
)
}
}

// Then trigger the listeners
if (notifyOptions.listeners) {
this.listeners.forEach((listener) => {
listener(this.#currentResult)
Expand Down
Loading