Maintain "status" variable in returned query state

[rodrigofariow]

Hi there!

First of all, great stuff being done here. I'm already a great fan of redux toolkit and this library has the potential to really help the redux world move forward, caching and data fetching wise so congrats! 🦾

I couldn't help but notice, though, that some public interfaces, like the one below, are leaning towards "impossible-state driven development".

For instance, I don't understand why variables like status below are getting deprecated and preference is being given to boolean flags.

https://rtk-query-docs.netlify.app/concepts/queries#query-hook-return-types

// Base query state
status: 'uninitialized'|'pending'|'fulfilled'|'rejected'; // @deprecated - A string describing the query state
originalArgs?: unknown; // Arguments passed to the query
internalQueryArgs?: unknown;
data?: unknown; // Returned result if present
error?: unknown; // Error result if present
requestId?: string; // A string generated by RTK Query
endpoint?: string; // The name of the given endpoint for the query
startedTimeStamp?: number; // Timestamp for when the query was initiatied
fulfilledTimeStamp?: number; // Timestamp for when the query was completed

isUninitialized: false; // Query has not started yet.
isLoading: false; // Query is currently loading for the first time. No data yet.
isFetching: false; // Query is currently fetching, but might have data from an earlier request.
isSuccess: false; // Query has data from a successful load.
isError: false; // Query is currently in "error" state.

refetch: () => void; // A function to force refetch the query

Although I understand that public boolean flags (e.g - isLoading, isError, isSuccess, etc) are here to provide "easy" access to our query state (and also that it doesn't mean your internal state actually uses them) I personally don't think this ideal.

In short, just like it's very easy to allow impossible states in a reducer's state when using isLoading, isSuccess, etc to represent the status of an endpoint call, I think it's also very easy to read this state in a way that it doesn't make sense, specially for junior devs.

e.g -

const { isUninitialized, isLoading } = useQuery(...)

if(isUninitialized && isLoading) {
  // Ooops
  return <Spinner />
}

Of course our integration tests can probably indicate that a spinner doesn't show when loading the query and most of our PR reviews will catch this but I would much rather not be able to do it, at all.

What are your thoughts on this?

Thank you for your time!

Cheers

[phryneas]

That's due to the fact that these status variables - while being kept in the reducers for the requests being fired - do not really correspond to what is going on in the component.

Stuff like coming from page 1 to page 2 will not make everything go back to uninitialized, you still have the data from page 1 available. It will be fetching, but not loading (while on the first request of page 1 you will be loading AND fetching), meaning you can show a loading indicator but might want to choose not to, still show the available data and maybe just gray it out.

All that cannot be indicated by a status variable, as that can only have one state - while the boolean flags can indicate multiple overlapping states at once.

[phryneas]

You can take a look at what states those can take here: https://github.com/rtk-incubator/rtk-query/blob/next/test/unionTypes.test.ts#L56

[rodrigofariow]

Thanks for the quick response @phryneas !

You made a good point by explaining those use cases.

And yeah, it does look like typescript type narrowing is working pretty well! 👍

I think I'm sorted now :)

Cheers!