Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 379670d

Browse files
authored
fix(core): make sure queries revert synchronously (#9601)
the switch from the callback approach to async/await with catch was necessary to be able to transform errors into revert-state data for imperative query calls; however, this was a change in behavior because catch in async/await doesn't happen immediately - it runs in the next microtask. That opened up an opportunity for a slight race condition if we re-start a fetch in-between. And who does that? Of course, React Strict Mode. This PR moves the actual state reverting back to a callback (so it happens synchronously), while still keeping the try/catch refactoring merely to transform the promise and the usual error handling
1 parent 428c19f commit 379670d

File tree

3 files changed

+64
-9
lines changed

3 files changed

+64
-9
lines changed

‎packages/query-core/src/__tests__/query.test.tsx

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,4 +1192,54 @@ describe('query', () => {
11921192
expect(initialDataFn).toHaveBeenCalledTimes(1)
11931193
expect(query.state.data).toBe('initial data')
11941194
})
1195+
1196+
test('should not override fetching state when revert happens after new observer subscribes', async () => {
1197+
const key = queryKey()
1198+
let count = 0
1199+
1200+
const queryFn = vi.fn(async ({ signal: _signal }) => {
1201+
// Destructure `signal` to intentionally consume it so observer-removal uses revert-cancel path
1202+
await sleep(50)
1203+
return 'data' + count++
1204+
})
1205+
1206+
const query = new Query({
1207+
client: queryClient,
1208+
queryKey: key,
1209+
queryHash: hashQueryKeyByOptions(key),
1210+
options: { queryFn },
1211+
})
1212+
1213+
const observer1 = new QueryObserver(queryClient, {
1214+
queryKey: key,
1215+
queryFn,
1216+
})
1217+
1218+
query.addObserver(observer1)
1219+
const promise1 = query.fetch()
1220+
1221+
await vi.advanceTimersByTimeAsync(10)
1222+
1223+
query.removeObserver(observer1)
1224+
1225+
const observer2 = new QueryObserver(queryClient, {
1226+
queryKey: key,
1227+
queryFn,
1228+
})
1229+
1230+
query.addObserver(observer2)
1231+
1232+
query.fetch()
1233+
1234+
await expect(promise1).rejects.toBeInstanceOf(CancelledError)
1235+
await vi.waitFor(() => expect(query.state.fetchStatus).toBe('idle'))
1236+
1237+
expect(queryFn).toHaveBeenCalledTimes(2)
1238+
1239+
expect(query.state).toMatchObject({
1240+
fetchStatus: 'idle',
1241+
status: 'success',
1242+
data: 'data1',
1243+
})
1244+
})
11951245
})

‎packages/query-core/src/query.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,15 @@ export class Query<
507507
| Promise<TData>
508508
| undefined,
509509
fn: context.fetchFn as () => Promise<TData>,
510-
abort: abortController.abort.bind(abortController),
510+
onCancel: (error) => {
511+
if (error instanceof CancelledError && error.revert) {
512+
this.setState({
513+
...this.#revertState,
514+
fetchStatus: 'idle' as const,
515+
})
516+
}
517+
abortController.abort()
518+
},
511519
onFail: (failureCount, error) => {
512520
this.#dispatch({ type: 'failed', failureCount, error })
513521
},
@@ -550,13 +558,9 @@ export class Query<
550558
if (error instanceof CancelledError) {
551559
if (error.silent) {
552560
// silent cancellation implies a new fetch is going to be started,
553-
// so we hatch onto that promise
561+
// so we piggyback onto that promise
554562
return this.#retryer.promise
555563
} else if (error.revert) {
556-
this.setState({
557-
...this.#revertState,
558-
fetchStatus: 'idle' as const,
559-
})
560564
// transform error into reverted state data
561565
// if the initial fetch was cancelled, we have no data, so we have
562566
// to get reject with a CancelledError

‎packages/query-core/src/retryer.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type { CancelOptions, DefaultError, NetworkMode } from './types'
1010
interface RetryerConfig<TData = unknown, TError = DefaultError> {
1111
fn: () => TData | Promise<TData>
1212
initialPromise?: Promise<TData>
13-
abort?: () => void
13+
onCancel?: (error: TError) => void
1414
onFail?: (failureCount: number, error: TError) => void
1515
onPause?: () => void
1616
onContinue?: () => void
@@ -86,9 +86,10 @@ export function createRetryer<TData = unknown, TError = DefaultError>(
8686

8787
const cancel = (cancelOptions?: CancelOptions): void => {
8888
if (!isResolved()) {
89-
reject(new CancelledError(cancelOptions))
89+
const error = new CancelledError(cancelOptions) as TError
90+
reject(error)
9091

91-
config.abort?.()
92+
config.onCancel?.(error)
9293
}
9394
}
9495
const cancelRetry = () => {

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /