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

❗ No coverage uploaded for pull request base (v5@096da93). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

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

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[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.