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: ensure hook subscription failures do not reset isLoading state #4364

Merged
merged 3 commits into from May 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/toolkit/src/query/react/buildHooks.ts
Expand Up @@ -687,10 +687,16 @@ export function buildHooks<Definitions extends EndpointDefinitions>({

const hasData = data !== undefined

// error is the last known error we have tracked - or if none has been tracked yet the last errored result for the current args
let error = currentState.isError ? currentState.error : lastResult?.error
if (error === undefined) error = currentState.error

const hasError = error !== undefined

// isFetching = true any time a request is in flight
const isFetching = currentState.isLoading
// isLoading = true only when loading while no data is present yet (initial load with no data in the cache)
const isLoading = !hasData && isFetching
const isLoading = !hasError && !hasData && isFetching
smacpherson64 marked this conversation as resolved.
Show resolved Hide resolved
// isSuccess = true when data is present
const isSuccess = currentState.isSuccess || (isFetching && hasData)

Expand Down
51 changes: 51 additions & 0 deletions packages/toolkit/src/query/tests/buildHooks.test.tsx
Expand Up @@ -873,6 +873,57 @@ describe('hooks tests', () => {
}
})

test('Hook subscription failures do not reset isLoading state', async () => {
const states: boolean[] = []

function Parent() {
const { isLoading } = api.endpoints.getUserAndForceError.useQuery(1)

// Collect loading states to verify that it does not revert back to true.
states.push(isLoading)

// Parent conditionally renders child when loading.
if (isLoading) return null

return <Child />
}

function Child() {
// Using the same args as the parent
api.endpoints.getUserAndForceError.useQuery(1)

return null
}

render(<Parent />, { wrapper: storeRef.wrapper })

// Allow at least three state effects to hit.
// Trying to see if any [true, false, true] occurs.
await act(async () => {
await waitMs(1)
})

await act(async () => {
await waitMs(1)
})

await act(async () => {
await waitMs(1)
})

// Find if at any time the isLoading state has reverted
// E.G.: `[..., true, false, ..., true]`
// ^^^^ ^^^^^ ^^^^
const firstTrue = states.indexOf(true)
const firstFalse = states.slice(firstTrue).indexOf(false)
const revertedState = states.slice(firstFalse).indexOf(true)

expect(
revertedState,
`Expected isLoading state to never revert back to true but did after ${revertedState} renders...`,
).toBe(-1)
})

describe('Hook middleware requirements', () => {
let mock: MockInstance

Expand Down