feat: remove keepPreviousData in favor of placeholderData

[DamianOsipiuk]

BREAKING CHANGE: removed keepPreviousData in favor of placeholderData identity function

Closes: #4688

@TkDodo Questions:

• do we want to default it to true? Meaning enabling this behavior by default with option to opt-out? I think we could make that switch later on if we decide to do that.
• do we expose identity function for users? I do not think it will bring too much, as users could use any identity function they have already in the project.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d6058fe:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-vue-basic	Configuration

[TkDodo]

do we want to default it to true

Tanner's poll wasn't very conclusive. I would keep the default placeholderData to undefined

do we expose identity function for users? I do not think it will bring too much, as users could use any identity function they have already in the project.

I would say exposing a function called keepPreviousData that is just an identity function might help with the migration (code-mod?). It would also be tree-shakeable if you don's use it, so there isn't a lot of drawback to expose it I think ?

[TkDodo]

very nice work 🚀

[codecov-commenter]

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (v5@096da93). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
packages/query-core/src/queryObserver.ts	50.00%	1 Missing and 1 partial ⚠️
packages/query-core/src/utils.ts	33.33%	2 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##             v5    #4715   +/-   ##
=====================================
  Coverage      ?   90.88%           
=====================================
  Files         ?       85           
  Lines         ?     3455           
  Branches      ?      889           
=====================================
  Hits          ?     3140           
  Misses        ?      294           
  Partials      ?       21           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[DamianOsipiuk]

@TkDodo @artysidorenko So when i tried to come up with identity function, it somehow messes up with the types returned from useQuery :(

// This works - const state: UseQueryResult<number, unknown>
const query1 = useQuery({
  queryKey: [key, count],
  queryFn: async () => {
    return Promise.resolve(count)
  },
  placeholderData: (previousData) => previousData,
})

// This is messed up - const query2: UseQueryResult<unknown, unknown>
function keepPreviousData<T>(previousData: T | undefined): T | undefined {
  return previousData
}
const query2 = useQuery({
  queryKey: [key, count],
  queryFn: async () => {
    return Promise.resolve(count)
  },
  placeholderData: keepPreviousData,
})

If i change this type definition from QueryObserverOptions:

placeholderData?: TQueryData | PlaceholderDataFunction<TQueryData>

to https://github.com/TanStack/query/pull/4715/files#diff-988cd43951b30185641ccb757eefd39c6e67027d7ccab1f445117b4c0401dc8bR251

type NonFunctionGuard<T> = T extends Function ? never : T

placeholderData?: NonFunctionGuard<TQueryData> | PlaceholderDataFunction<NonFunctionGuard<TQueryData>>

It works as intended.

Therefore i assume that Typescript might be confused cause in TQueryData | PlaceholderDataFunction<TQueryData> if fails to narrow down the type of TQueryData to not be a function and therefore falls back to unknown.
Not sure if this is another TS bug or limitation.

What do you think about this solution?

[TkDodo]

changes look good to me - thanks for also using keepPreviousData internally for our tests 🎉

[TkDodo]

oh, there are conflicts because of the overloads removal 😅

[TkDodo]

can you please resolve the conflicts here, too?

[DamianOsipiuk]

can you please resolve the conflicts here, too?

Done ✅

[jyuloeng]

Hi @TkDodo ~ Sorry to ask, but my project relies heavily on keepPreviousData now, so in v5, should I use such as

const query1 = useQuery({
  queryKey: [key, count],
  queryFn: async () => {
    return Promise.resolve(count)
  },
  placeholderData: (previousData) => previousData,
})

this method instead?

[TkDodo]

@jyuloeng yes that's correct. For convenience, you can also do:

import { placeholderData } from '@tanstack/react-query`

placeholderData: keepPreviousData

it's just an identity function that we are exporting.