Skip to content

fix(query-core): prevent state override when observer remount occurs … #9580

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
87 changes: 87 additions & 0 deletions packages/query-core/src/__tests__/query.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1192,4 +1192,91 @@ describe('query', () => {
expect(initialDataFn).toHaveBeenCalledTimes(1)
expect(query.state.data).toBe('initial data')
})

test('should not override fetching state when revert happens after new observer subscribes', async () => {
const key = queryKey()

const queryFn = vi.fn(async ({ signal: _signal }) => {
// Destructure `signal` to intentionally consume it so observer-removal uses revert-cancel path
await sleep(50)
return 'data'
})

const query = new Query({
client: queryClient,
queryKey: key,
queryHash: hashQueryKeyByOptions(key),
options: { queryFn },
})

const observer1 = new QueryObserver(queryClient, {
queryKey: key,
queryFn,
})

query.addObserver(observer1)
const promise1 = query.fetch()

await vi.advanceTimersByTimeAsync(10)

query.removeObserver(observer1)

const observer2 = new QueryObserver(queryClient, {
queryKey: key,
queryFn,
})

query.addObserver(observer2)

query.fetch()

await expect(promise1).rejects.toBeInstanceOf(CancelledError)
await vi.waitFor(() => expect(query.state.fetchStatus).toBe('fetching'))

expect(query.state.fetchStatus).toBe('fetching')
})

test('should throw CancelledError when revert happens with no data after observer removal', async () => {
const key = queryKey()

const queryFn = vi.fn(async ({ signal: _signal }) => {
// Destructure `signal` to intentionally consume it so observer-removal uses revert-cancel path
await sleep(50)
return 'data'
})

const query = new Query({
client: queryClient,
queryKey: key,
queryHash: hashQueryKeyByOptions(key),
options: { queryFn },
})

const observer1 = new QueryObserver(queryClient, {
queryKey: key,
queryFn,
})

query.addObserver(observer1)
const promise1 = query.fetch()

await vi.advanceTimersByTimeAsync(5)

query.removeObserver(observer1)

const observer2 = new QueryObserver(queryClient, {
queryKey: key,
queryFn,
})

query.addObserver(observer2)
query.fetch()

await expect(promise1).rejects.toThrow(CancelledError)

expect(query.state.fetchStatus).toBe('fetching')

await vi.advanceTimersByTimeAsync(50)
expect(query.state.data).toBe('data')
})
})
17 changes: 13 additions & 4 deletions packages/query-core/src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ export class Query<
// we'll let the query continue so the result can be cached
if (this.#retryer) {
if (this.#abortSignalConsumed) {
this.#retryer.cancel({ revert: true })
this.#retryer.cancel({ revert: true, isObserverRemoval: true })
} else {
this.#retryer.cancelRetry()
}
Expand Down Expand Up @@ -553,16 +553,25 @@ export class Query<
// so we hatch onto that promise
return this.#retryer.promise
} else if (error.revert) {
// If cancellation was caused by observer removal and there are active observers again,
// do not revert to idle: a new fetch may already be in flight, and reverting would
// incorrectly flip isLoading/isFetching to false under StrictMode remounts.
if (error.isObserverRemoval && this.isActive()) {
if (this.state.data === undefined) {
throw error
}
return this.state.data
}

this.setState({
...this.#revertState,
fetchStatus: 'idle' as const,
})
// transform error into reverted state data
// if the initial fetch was cancelled, we have no data, so we have
// to get reject with a CancelledError

if (this.state.data === undefined) {
throw error
}

return this.state.data
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/query-core/src/retryer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ export function canFetch(networkMode: NetworkMode | undefined): boolean {
export class CancelledError extends Error {
revert?: boolean
silent?: boolean
isObserverRemoval?: boolean
constructor(options?: CancelOptions) {
super('CancelledError')
this.revert = options?.revert
this.silent = options?.silent
this.isObserverRemoval = options?.isObserverRemoval
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/query-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,7 @@ export interface DefaultOptions<TError = DefaultError> {
export interface CancelOptions {
revert?: boolean
silent?: boolean
isObserverRemoval?: boolean
}

export interface SetDataOptions {
Expand Down
Loading