isSuccess flashes false on subsequent queries

[kellengreen]

Given a basic component like the one below, response.isSuccess flashes false on subsequent queries.

Basic Example

import { useState } from "react";
import api from "./api";

export default function App() {
  const [params, setParams] = useState({ req: 0 });
  const response = api.useSearchQuery(params);

  console.log(
    `request: ${params.req}\tisFetching: ${response.isFetching}\tisSuccess: ${response.isSuccess}`
  );

  return (
    <div>
      <button
        disabled={response.isFetching}
        onClick={() => setParams({ req: params.req + 1 })}
      >
        Refresh
      </button>
      <pre>{JSON.stringify(response, undefined, 3)}</pre>
    </div>
  );
}

Current Behavior in Console Log

request: 0	isFetching: true	isSuccess: false
request: 0	isFetching: true	isSuccess: false
request: 0	isFetching: false	isSuccess: true
request: 1	isFetching: true	isSuccess: false <--
request: 1	isFetching: true	isSuccess: true
request: 1	isFetching: false	isSuccess: true

Expected Behavior in Console Log

Given this quote from the documentation:

isSuccess - When true, indicates that the query has data from a successful request.

I would expect isSuccess to remain true while data is not undefined.

request: 0	isFetching: true	isSuccess: false
request: 0	isFetching: true	isSuccess: false
request: 0	isFetching: false	isSuccess: true
request: 1	isFetching: true	isSuccess: true <--
request: 1	isFetching: true	isSuccess: true
request: 1	isFetching: false	isSuccess: true

Repo to Reproduce: https://github.com/kellengreen/rtk

[kellengreen]

Anyone?

[markerikson]

Sorry, tbh we're all pretty busy with things lately and haven't had any time to try looking into assorted bugs.

[kellengreen]

No worries, I understand.

[kellengreen]

Confirmed still an issue in @reduxjs/toolkit = 2 and react-redux = 9.

[CO0Ki3]

@markerikson We should look at the definition of isSuccess in the values in 'initialized' state.

const isSuccess = currentState.isSuccess || isFetching && hasData;

If the state is 'initialized', isFetching(isLoading) is false, so isSuccess is false even if hasData is true (when it has previous data).

Can I ask why isFetching & hasData condition is necessary?

[riqts]

@markerikson We should look at the definition of isSuccess in the values in 'initialized' state.

const isSuccess = currentState.isSuccess || isFetching && hasData;

If the state is 'initialized', isFetching(isLoading) is false, so isSuccess is false even if hasData is true (when it has previous data).

Can I ask why isFetching & hasData condition is necessary?

Lenz discusses the motivation a bit here

You likely don't want isSuccess fulfilled if no query has been fulfilled. Just having data may now be sufficient, like when using an optimistic update.

Although in saying that, I don't think const isSuccess = currentState.isSuccess || isFetching && hasData; is a sufficient enough check.

Likely a better check could be as simple as const isSuccess = currentState.isSuccess || (currentState.startedTimestamp/fulfilledTimestamp) && hasData;

But I haven't had a chance to test this just thinking about what's available

[kellengreen]

Sorry the thread is a lot to digest, do we now agree that this is unintended behavior?

[riqts]

Sorry the thread is a lot to digest, do we now agree that this is unintended behavior?

The flashing of the isSuccees is almost certainly unintended.

The necessary context here is just knowing isSuccess represents more than just whether data exists - it also represents that data exists from a successful query.

I haven't tested whether just changing the check in buildHooks.ts from isFetching && hasData to something more query specific like currentState.fulfilledTimestamp is enough to fix this. The other possibility, in my mind, is that the state pre selector is being triggered again at the same time during the refetch.

[kellengreen]

Understood, thank you. The flashing is all I really care about. In our app it lead to layout shifting during searches which obviously didn't look great.

[CO0Ki3]

@riqts

The flashing of the isSuccees is almost certainly unintended.

I have experienced the flashing of a feature that corresponds to isSuccess in other famous query libraries in my work. It would be nice to recreate that case and see how the other libraries have written the conditions. I don't know the deadline for sure, but I'll try my best😅

[markerikson]

Looking into this and have a test that appears to replicate the behavior.

test.only('`isSuccess` does not jump back false on subsequent queries', async () => {
      const successHist: boolean[] = [],
        fetchingHist: boolean[] = []

      function User({ id }: { id: number }) {
        const queryRes = api.endpoints.getUser.useQuery(id)

        const { isSuccess, isFetching, status } = queryRes

        console.log(
          `id: ${id}\tisFetching: ${queryRes.isFetching}\tisSuccess: ${queryRes.isSuccess}`,
        )

        useEffect(() => {
          successHist.push(isSuccess)
        }, [isSuccess])
        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(successHist).toEqual([false, true, true, true, true])
      expect(fetchingHist).toEqual([true, false, true, true, false])
    })

and per above, here's the logged output:

id: 1   isFetching: true        isSuccess: false
id: 1   isFetching: true        isSuccess: false
id: 1   isFetching: false       isSuccess: true
id: 2   isFetching: true        isSuccess: false <-----
id: 2   isFetching: true        isSuccess: true
id: 2   isFetching: false       isSuccess: true

Here's the values in scope in the query selector at the time of that {id: 2, isFetching: true, isSuccess: false} line:

{
  currentState: {
    status: 'uninitialized',
    isUninitialized: true,
    isLoading: false,
    isSuccess: false,
    isError: false
  },
  lastResult: {
    status: 'fulfilled',
    endpointName: 'getUser',
    requestId: 'QWSeI6-xRVNKXYeV0Zc0a',
    originalArgs: 1,
    startedTimeStamp: 1732400449088,
    data: { name: 'Timmy' },
    fulfilledTimeStamp: 1732400449239,
    isUninitialized: false,
    isLoading: false,
    isSuccess: true,
    isError: false,
    currentData: { name: 'Timmy' },
    isFetching: false
  },
  queryArgs: 2,
  data: { name: 'Timmy' },
  hasData: true,
  isFetching: false,
  isLoading: false,
  isSuccess: false
}

given that the logic is const isSuccess = currentState.isSuccess || (isFetching && hasData), it seems that the problem is that we have isFetching: false at that time.

on the flip side, we also have:

const noPendingQueryStateSelector: QueryStateSelector<any, any> = (
  selected,
) => {
  if (selected.isUninitialized) {
    return {
      ...selected,
      isUninitialized: false,
      isFetching: true,
      isLoading: selected.data !== undefined ? false : true,
      status: QueryStatus.pending,
    } as any
  }
  return selected
}

so it feels like we have a mismatch here between what logic we're using where.

[markerikson]

Out in https://github.com/reduxjs/redux-toolkit/releases/tag/v2.4.0 !