regarding isLoading for disabled queries in v4

[vemoo]

I encountered this issue: #3584, and I fixed it by using isFetching.

But I don't understand the rationale and I think it violates the principle of least surprise. If it's to only have 3 states, wouldn't it be better for disabled queries to be in the success state instead of the loading state?

Also there are a few places on the documentation that I think contradict this change:

• in https://tanstack.com/query/v4/docs/reference/useQuery in status: String it says:

  loading if the query is in a "hard" loading state. This means there is no cached data and the query is currently fetching, eg isFetching === true

• in https://tanstack.com/query/v4/docs/guides/queries

  For most queries, it's usually sufficient to check for the isLoading state, then the isError state, then finally, assume that the data is available and render the successful state:

  I would argue that for most queries isFetching behavior is desired

[TkDodo]

I would argue that for most queries isFetching behavior is desired

isFetching will also be true for background refetches, so if you do:

if (isFetching) {
  return "loading..."
}
return <RestOfTheApp />

you will unmount your app and replace it with a loading spinner on every background refetch (e.g. a window focus after staleTime is reached), which is most likely not what you want. So no, it's likely not the desired behaviour.

wouldn't it be better for disabled queries to be in the success state instead of the loading state?

the status defines and discriminates the data property. success means data is available, error means data is not available, but error is, and loading means data is not yet available. It's equivalent to the 3 states a promise can be in: pending, resolved or rejected.

disabled queries are kind of a hybrid - they don't have data, but they are also not currently getting any data. That's why we previously had the idle state, but it complicated a lot of things, like exhaustive matching, and it also introduced impossible states (while you're idle, you cannot be isFetching: true). Now, all combinations of status and fetchStatus are possible.

Also, disabled queries can be in success state. For example, if you use initialData, placeholderData, or got data into the query cache by some other means, the query will be in success state, because success just means "data is available". enabled: false basically only means: this instance of useQuery will not fetch any data, nothing more. Of course, if it is the only instance of useQuery (or all are disabled), and you don't have data yet, you will be in this in-between state.

So my question would be: what are you currently rendering in case of isIdle?

• For dependent queries, users often show a loading spinner until both queries have data, which you can still do now with an isLoading check.
• For queries that are disabled until the user has made some input, often null is shown in these cases. If you do that, checking for data availability ( e.g. if (data)) might be the best way.

If you want a 1:1 replacement of checks, the migration guide shows how to achieve that:

 - status: 'idle'
 + status: 'loading'
 + fetchStatus: 'idle'

so you would need to check for isLoading && fetchStatus === 'idle'

[vemoo]

Wouldn't it make sense for isLoading to be defined as status === 'loading' && fetchStatus !== 'idle' then? Because isn't that the most common scenario a user will want to know about?

I'm using it for queries that are disabled until the user has made some input (a search form). But if use the pattern that is used in a lot o the examples:

if (isLoading) return "Loading...";

or something like

<button disabled={isLoading}>Search</button>

that won't work because the search form UI will never be rendered or the search button will always be disabled.

I understand how to fix it, but from a developer experience point of view it's not great, and I think it can be a frequent stumbling block.

[TkDodo]

Wouldn't it make sense for isLoading to be defined as status === 'loading' && fetchStatus !== 'idle' then?

we could do that, but right now, isLoading is just the same as status === 'loading'; that's the same for isError and isSuccess - they are just derived from the status field.

for disabling a button, as you said, isFetching is likely what you want, unless clicking the button while a background refetch is happening is okay?

the examples that use isLoading do not show disabled queries. I'm thinking that if (isLoading) return "Loading..."; would not be ideal for disabled queries when you are waiting for user input to enable it, because that would mean that you would show some kind of input field, then enable the query, then unmount that input field to show a spinner ...

I understand how to fix it, but from a developer experience point of view it's not great, and I think it can be a frequent stumbling block.

the biggest source of confusion is probably naming. If isLoading were renamed to isPending, maybe it would be less confusing? Because to me, it would indicate that the data is not there yet - pending - which doesn't mean that we are currently "getting" it. loading has the meaning of a "loading spinner", so yeah, I understand why that is confusing.

[vemoo]

Yeah, I agree with a name like isPending it would be less confusing.

PS: I now see that now one can do:

const { status, data } = useQuery(["todo"], () => fetchTodo());
if (status === "success") {
    // data is not undefined here!
    data.length
}

Which is really cool!

[TkDodo]

yes, that's the type narrowing / discrimination I was talking about. I've also re-written the typescript docs recently to better reflect this:

https://tanstack.com/query/v4/docs/typescript