placeholderData not applied when query is disabled

[timtnleeProject]

Describe the bug
When there're query dependent, the last query:

1. placeholderData not applied until the query is fetching.
2. isLoading never triggerd.

To Reproduce
online sandbox:
https://codesandbox.io/s/react-query-issue-et8hs?file=/src/index.js

I have two querys: auth, getUserDetail.
getUserDetail depends on auth.

The fetching order will be: auth -> getUserDetail.

placeholderData of getUserDetail is set to "placeholder"

1. when auth is fetching, the placeholderData of getUserDetail is undefined (BUG ?).
2. when getUserDetail is fetching, the placeholderData of getUserDetail is "placeholder" .
3. isLoading of getUserDetail is never been true (BUG ? I want to make sure this is expect behavior.).

Expected behavior

1. placeholderData applied at start (isIdle).

Desktop (please complete the following information):

• Device: [PC]
• OS: [Win10]
• Browser: [edge(chromium)]

Additional context

[boschni]

Do we also want to show the placeholder data when a query is not enabled @tannerlinsley?

[TkDodo]

Do we also want to show the placeholder data when a query is not enabled @tannerlinsley?

Good question. Since placeholder data is meant to be “incomplete” data that never goes into the cache, i would say yes. How is initialData handling this case? I think it also works on disabled queries, but I’m not sure it should 🤔 See also here maybe: #1318

[tannerlinsley]

Generically, I would expect placeholderData to populate and function as expected on disabled queries and initialData to not.

[mwisnicki]

I also expected placeholderData to be returned in this case.
BTW once you do that you can drop unknown from TData when placeholder is set.

[TkDodo]

Generically, I would expect placeholderData to populate and function as expected on disabled queries and initialData to not.

@tannerlinsley since initialData is persisted to the cache, we currently apply it when the queryCache entry is created, which also happens for disabled queries. Also, a query can't really be enabled or disabled, only observers can (you can have two observers on the same query, one being enabled and one disabled). So I'm not sure if this would be doable, and it also goes a bit against my understanding of initialData...

I also expected placeholderData to be returned in this case.

Yes it makes sense. Would you like to take a stab at it?

BTW once you do that you can drop unknown from TData when placeholder is set.

You likely mean undefined, and that is not an easy thing to do, because you can define all options also on the QueryClient as defaults, or via queryClient.setQueryDefaults for a specific query. Ignoring this, and only making it work on type level for useQuery directly, you still have the case where you define placeholderData, but the query that fetches in the background errors. In this case, the query will go to error state and placeholderData will no longer be applied, so you're back to undefined data. I think only initialData can solve this, and we could take a stab at improving the types there.

[TkDodo]

could it be as easy as adding || status === 'idle' here after we check for loading state?

https://github.com/tannerlinsley/react-query/blob/69b0c9857c04ab77eb394c416454650ba5963fc9/src/core/queryObserver.ts#L494-L499

🤔

[mwisnicki]

you still have the case where you define placeholderData, but the query that fetches in the background errors. In this case, the query will go to error state and placeholderData will no longer be applied, so you're back to undefined data. I think only initialData can solve this, and we could take a stab at improving the types there.

IMHO placeholderData should be returned in all cases where there is no valid data. That includes errors.
Such behavior would match basic implementation of of fetching with state hooks.

[TkDodo]

IMHO placeholderData should be returned in all cases where there is no valid data. That includes errors.
Such behavior would match basic implementation of of fetching with state hooks.

imo that is what initialData is for. placeholderData moves the loading query directly to success state and we can’t do that in case of error. It would be a substantial change to how placeholderData works

[mwisnicki]

Are you sure? How can placeholderData move query to success if it is supposed to be returned while query is executing (therefore no success yet)?

[TkDodo]

Well internally, it stays in loading because placeholderData only concerns the observer that uses it. But when placeholderData is supplied, the status on the returned result will be success. This happens here:

https://github.com/tannerlinsley/react-query/blob/a7780444bb0bdd0e7a768c6ebc57e04aea5f7c51/src/core/queryObserver.ts#L533-L538

[mwisnicki]

Seems very counterintuitive

[TkDodo]

What alternative would you suggest? We can’t keep it in loading state with data because loading state implies that you have no data. placeholderData is a "fake" success because it gives you data so that you can fake-it-till-you-make-it on screen. And if the query fails you go to error with no data, because you didn’t make it.