From 86bb20a5c24a6e1afe81f8ce806ea856038f3c1c Mon Sep 17 00:00:00 2001 From: smacpherson64 Date: Mon, 22 Apr 2024 16:20:42 -0500 Subject: [PATCH 1/3] fix: ensure hook subscription failures do not reset isLoading state --- .../toolkit/src/query/react/buildHooks.ts | 8 ++- .../src/query/tests/buildHooks.test.tsx | 51 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts index d2432acc93..b9dc7f54a8 100644 --- a/packages/toolkit/src/query/react/buildHooks.ts +++ b/packages/toolkit/src/query/react/buildHooks.ts @@ -687,10 +687,16 @@ export function buildHooks({ 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 // isSuccess = true when data is present const isSuccess = currentState.isSuccess || (isFetching && hasData) diff --git a/packages/toolkit/src/query/tests/buildHooks.test.tsx b/packages/toolkit/src/query/tests/buildHooks.test.tsx index 2d5106971c..32c5558394 100644 --- a/packages/toolkit/src/query/tests/buildHooks.test.tsx +++ b/packages/toolkit/src/query/tests/buildHooks.test.tsx @@ -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 + } + + function Child() { + // Using the same args as the parent + api.endpoints.getUserAndForceError.useQuery(1) + + return null + } + + render(, { 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 From 4f2e8aa2001dbc337aaaa731f45e96feb8435221 Mon Sep 17 00:00:00 2001 From: smacpherson64 Date: Mon, 22 Apr 2024 17:03:36 -0500 Subject: [PATCH 2/3] Update packages/toolkit/src/query/react/buildHooks.ts Co-authored-by: Lenz Weber-Tronic --- packages/toolkit/src/query/react/buildHooks.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts index b9dc7f54a8..4b03031841 100644 --- a/packages/toolkit/src/query/react/buildHooks.ts +++ b/packages/toolkit/src/query/react/buildHooks.ts @@ -696,7 +696,7 @@ export function buildHooks({ // 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 = !hasError && !hasData && isFetching + const isLoading = (!lastResult || lastResult.isLoading || lastResult.isUninitialized) && !hasData && isFetching // isSuccess = true when data is present const isSuccess = currentState.isSuccess || (isFetching && hasData) From e5a9962f45b7c6b2e8e57e413e13a891e9a89ec4 Mon Sep 17 00:00:00 2001 From: smacpherson64 Date: Mon, 22 Apr 2024 17:05:31 -0500 Subject: [PATCH 3/3] removes unused error code --- packages/toolkit/src/query/react/buildHooks.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts index 4b03031841..86de0e11b4 100644 --- a/packages/toolkit/src/query/react/buildHooks.ts +++ b/packages/toolkit/src/query/react/buildHooks.ts @@ -687,12 +687,6 @@ export function buildHooks({ 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)