fix "isLoading briefly flips back to true" #1519 (#1520)

Author: phryneas

packages/toolkit/src/query/react/buildHooks.ts

@@ -406,16 +406,17 @@ const queryStatePreSelector = (
   lastResult: UseQueryStateDefaultResult<any>
 ): UseQueryStateDefaultResult<any> => {
   // data is the last known good request result we have tracked - or if none has been tracked yet the last good result for the current args
-  const data =
-    (currentState.isSuccess ? currentState.data : lastResult?.data) ??
-    currentState.data
+  let data = currentState.isSuccess ? currentState.data : lastResult?.data
+  if (data === undefined) data = currentState.data
+
+  const hasData = data !== 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 = !data && isFetching
+  const isLoading = !hasData && isFetching
   // isSuccess = true when data is present
-  const isSuccess = currentState.isSuccess || (isFetching && !!data)
+  const isSuccess = currentState.isSuccess || (isFetching && hasData)
 
   return {
     ...currentState,
@@ -440,7 +441,7 @@ const noPendingQueryStateSelector: QueryStateSelector<any, any> = (
       ...selected,
       isUninitialized: false,
       isFetching: true,
-      isLoading: true,
+      isLoading: selected.data !== undefined ? false : true,
       status: QueryStatus.pending,
     } as any
   }

packages/toolkit/src/query/tests/buildHooks.test.tsx

@@ -44,9 +44,7 @@ const api = createApi({
     }
 
     return {
-      data: arg?.body
-        ? { ...arg.body, ...(amount ? { amount } : {}) }
-        : undefined,
+      data: arg?.body ? { ...arg.body, ...(amount ? { amount } : {}) } : {},
     }
   },
   endpoints: (build) => ({
@@ -146,16 +144,20 @@ describe('hooks tests', () => {
     })
 
     test('useQuery hook sets isLoading=true only on initial request', async () => {
-      let refetch: any, isLoading: boolean
+      let refetch: any, isLoading: boolean, isFetching: boolean
       function User() {
         const [value, setValue] = React.useState(0)
 
-        ;({ isLoading, refetch } = api.endpoints.getUser.useQuery(2, {
-          skip: value < 1,
-        }))
+        ;({ isLoading, isFetching, refetch } = api.endpoints.getUser.useQuery(
+          2,
+          {
+            skip: value < 1,
+          }
+        ))
         return (
           <div>
             <div data-testid="isLoading">{String(isLoading)}</div>
+            <div data-testid="isFetching">{String(isFetching)}</div>
             <button onClick={() => setValue((val) => val + 1)}>
               Increment value
             </button>
@@ -182,14 +184,12 @@ describe('hooks tests', () => {
       await waitFor(() =>
         expect(screen.getByTestId('isLoading').textContent).toBe('false')
       )
-      // We call a refetch, should set to true
+      // We call a refetch, should still be `false`
       act(() => refetch())
       await waitFor(() =>
-        expect(screen.getByTestId('isLoading').textContent).toBe('true')
-      )
-      await waitFor(() =>
-        expect(screen.getByTestId('isLoading').textContent).toBe('false')
+        expect(screen.getByTestId('isFetching').textContent).toBe('true')
       )
+      expect(screen.getByTestId('isLoading').textContent).toBe('false')
     })
 
     test('useQuery hook sets isLoading and isFetching to the correct states', async () => {
@@ -240,10 +240,10 @@ describe('hooks tests', () => {
       })
       expect(getRenderCount()).toBe(5)
 
-      // We call a refetch, should set both to true, then false when complete/errored
+      // We call a refetch, should set `isFetching` to true, then false when complete/errored
       act(() => refetchMe())
       await waitFor(() => {
-        expect(screen.getByTestId('isLoading').textContent).toBe('true')
+        expect(screen.getByTestId('isLoading').textContent).toBe('false')
         expect(screen.getByTestId('isFetching').textContent).toBe('true')
       })
       await waitFor(() => {
@@ -253,6 +253,42 @@ describe('hooks tests', () => {
       expect(getRenderCount()).toBe(7)
     })
 
+    test('`isLoading` does not jump back to true, while `isFetching` does', async () => {
+      const loadingHist: boolean[] = [],
+        fetchingHist: boolean[] = []
+
+      function User({ id }: { id: number }) {
+        const { isLoading, isFetching, status } =
+          api.endpoints.getUser.useQuery(id)
+
+        React.useEffect(() => {
+          loadingHist.push(isLoading)
+        }, [isLoading])
+        React.useEffect(() => {
+          fetchingHist.push(isFetching)
+        }, [isFetching])
+        return (
+          <div data-testid="status">
+            {status === QueryStatus.fulfilled && id}
+          </div>
+        )
+      }
+
+      let { rerender } = render(<User id={1} />, { wrapper: storeRef.wrapper })
+
+      await waitFor(() =>
+        expect(screen.getByTestId('status').textContent).toBe('1')
+      )
+      rerender(<User id={2} />)
+
+      await waitFor(() =>
+        expect(screen.getByTestId('status').textContent).toBe('2')
+      )
+
+      expect(loadingHist).toEqual([true, false])
+      expect(fetchingHist).toEqual([true, false, true, false])
+    })
+
     test('useQuery hook respects refetchOnMountOrArgChange: true', async () => {
       let data, isLoading, isFetching
       function User() {
@@ -903,7 +939,7 @@ describe('hooks tests', () => {
       expect(
         api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any)
       ).toEqual({
-        data: undefined,
+        data: {},
         endpointName: 'getUser',
         error: undefined,
         fulfilledTimeStamp: expect.any(Number),
@@ -924,7 +960,7 @@ describe('hooks tests', () => {
       expect(
         api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any)
       ).toEqual({
-        data: undefined,
+        data: {},
         endpointName: 'getUser',
         fulfilledTimeStamp: expect.any(Number),
         isError: false,
@@ -972,7 +1008,7 @@ describe('hooks tests', () => {
       expect(
         api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any)
       ).toEqual({
-        data: undefined,
+        data: {},
         endpointName: 'getUser',
         fulfilledTimeStamp: expect.any(Number),
         isError: false,
@@ -990,7 +1026,7 @@ describe('hooks tests', () => {
       expect(
         api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any)
       ).toEqual({
-        data: undefined,
+        data: {},
         endpointName: 'getUser',
         fulfilledTimeStamp: expect.any(Number),
         isError: false,
@@ -1040,7 +1076,7 @@ describe('hooks tests', () => {
       expect(
         api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any)
       ).toEqual({
-        data: undefined,
+        data: {},
         endpointName: 'getUser',
         fulfilledTimeStamp: expect.any(Number),
         isError: false,
@@ -1060,7 +1096,7 @@ describe('hooks tests', () => {
       expect(
         api.endpoints.getUser.select(USER_ID)(storeRef.store.getState() as any)
       ).toEqual({
-        data: undefined,
+        data: {},
         endpointName: 'getUser',
         fulfilledTimeStamp: expect.any(Number),
         isError: false,