feat: add placeholderData to queryObserver

[tannerlinsley]

Trying my hand at a bit of Typescript! I feel so out of my element here... but hopefully not for long. 😉

This PR is clearly incomplete, but actually works as a solution in my codesandbox from #1158.

Thoughts here?

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tannerlinsley/react-query/27a9g3ka3
✅ Preview: https://react-query-git-feat-placeholder-data.tannerlinsley.vercel.app

[boschni]

Great to see you are digging into it! I kind of like the placeholderData but do have two questions:

1. What are the benefits over using:

const { data = 'initial' } = useQuery()

2. The success status does not feel correct to me as the query has never been completed. It removes a bit of insight into the actual status of the query. Maybe it should be loading while having data?

[tannerlinsley]

1

• You don't have to destructure to get the ease of setting the default.
• You can hoist the placeholder data into a custom hook or configuration object, which is way more
• composable/portable than scripting into the default value at every usage
• It does not take care of the added state overrides needed to display the data.

2

• Keep in mind, we're not changing the status of the query, just the observers interpretation of the query, much like isStale.
• The data you're providing should be roughly compatible with any actual data you're displaying, so isSuccess is a pretty safe bet, especially when you consider that the user is opting into the placeholder data. Similar to initialData they can and should expect it to be treated like normal data while it's being filled.
• Adding yet another state to the already complicates existing template flows for almost every user. isIdle was already difficult enough for most people to handle when we introduced it, and I think this would similarly add a weird step for users where it's unnecessary. eg.

return query.isLoading ? 'Loading' : query.isError ? 'Error' : 'Show Data JSX'

// Becomes

return query.isPlaceholder
  ? 'Show Successful JSX'
  : query.isLoading
  ? 'Loading'
  : query.isError
  ? 'Error'
  : 'Show Successful JSX'

Empirically, I can see how adding another state would make technical sense, but where successful and placeholder states are (and should be) almost identical in templating logic, I see more negatives (breaking everyone's templates who uses it) with introducing the new state.

[boschni]

Yeah a placeholderData option can indeed provide some additional convenience. I'm in favor of adding it. My main doubt is about the status property, which does not really seem to indicate the query has executed successfully anymore. It's becoming more of a flag to indicate data is not undefined instead of reflecting the current query status. It's one of the reasons why I think why value checking is more flexible and correct:

const { data } = useQuery()

if (data) {
  return <div>{data}</div>
}

If you also want to know what kind of data it is:

const { data, isPlaceholderData } = useQuery()

if (data) {
  return <div className={isPlaceholderData ? 'loading' : ''}>{data}</div>
}

[tannerlinsley]

Hmmm. That would create a lot of upheaval from the current documentation, but if it's the right thing to do, we should discuss it thoroughly to make sure we're making the right move. It would be a relatively large change conceptually from what it's been in the past.

[tannerlinsley]

I think the whole data checking approach fits this situation pretty well, but it's when we get into showing/persisting errors and data that the approach rubs me the wrong way.

[tannerlinsley]

From a maintainer perspective, it costs us very little to just keep things the way they are and add the feature. It doesn't break anyone in v2 or v3 and it's pretty clear what the option is doing in the docs. We can always reformat things in the future, but my guess is that no one would even bat an eyelash at the placeholder/isSuccess combo for quite a long time.

In contrast, changing everything to be data/error/loading checking as a primary means of templating would be a ton of work today and probably create a lot of churn for little payoff IMO.

[boschni]

Yeah I agree let's keep the value/status checking discussion in #1108. It should not be blocking this feature. Wouldn't a isPlaceholderData flag be handy if you want to show a loader or maybe a bit different design while rendering placeholder data?

[tannerlinsley]

Added

[tannerlinsley]

I'm guessing this will need to be manually added to v3 as well.

[tannerlinsley]

🎉 This PR is included in version 3.2.0-beta.4 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[jxe]

It's not clear from the docs if setting placeholderData means the hook won't suspend even when isLoading and suspense are both true.