placeholderData is not transformed by select

[drakang4]

Describe the bug

placeholderData passed to useQuery's option is not transformed by select when it exists.

To Reproduce

https://codesandbox.io/s/react-query-placeholderdata-with-select-dsvxl

data.toString() renders 2020-02-18T15:29:59.326Z at first.
After 1 second, it renders formatted date like Fri Feb 19 2021 00:29:59 GMT+0900 as data is transformed to Date object by select function.

Then replace placeholderData to initialData. It shows Wed Feb 19 2020 00:29:59 GMT+0900 and 1 second later Fri Feb 19 2021 00:29:59 GMT+0900 as both are Date object.

Expected behavior

It seems placeholderData should be transformed by select to keep return type of data like initialData.

[essential-randomness]

Piggybacking on this issue with a related problem:

• The type of placeholderData is placeholderData?: TData | PlaceholderDataFunction<TData>
• The type of initialData is initialData?: TData | InitialDataFunction<TData>
• The type of select is select?: (data: TQueryData) => TData

I don't know whether the behavior of initialData (call select on the data) or placeholderData (don't call select on the data) is the intended one, but it seems like the types are wrong. If select is being called on either, their type should be TQueryData.

[Svish]

This is what I hoped would work, but it didn't, because of this issue:

// api.ts
export interface Company { id: number; name: string; }

export function useGetCompanies<R = Company[]>(select?: (data: Company[]) => R): UseQueryResult<R> {
  return useQuery(['api', 'companies'], fetchCompanies, {
    select,
    placeholderData: () =>
      localStorage.getItem<Company[]>('companies') ?? undefined,
    onSuccess: (data) => {
      localStorage.setItem<Company[]>('companies', data);
    },
  });
}

// In some component
const selectOptions = useGetCompanies(
  data => data.map(company => ({ value: company.id, label: company.name }))
);

Works the first time, as the response is stored in localStorage, and the component gets the mapped options via select. But second time around, the component instead gets the original unmapped response stored in localStorage, which of course causes... issues...

Basically, using placeholderData and select together in "wrapper hooks" like this, is currently not really possible. One either needs to not use placeholderData, or not use select. 😕

[Svish]

I don't know whether the behavior of initialData (call select on the data) or placeholderData (don't call select on the data) is the intended one, but it seems like the types are wrong. If select is being called on either, their type should be TQueryData.

With current behaviour of placeholderData, I would say the type is correct. But I'm wondering if initialData is actually wrong since it according to OP does run its data through select. I'm thinking the types of these probably should be the following:

    data: TData | undefined;
    select?: (data: TQueryData) => TData;
    initialData?: TQueryData | InitialDataFunction<TQueryData>;
    placeholderData?: TQueryData | PlaceholderDataFunction<TQueryData>;

Where both initialData and placeHolderData passes the TQueryData through select before it ends up as TData.

[TkDodo]

So I see multiple issues here, let's all get on the same page first, my baseline example is the following:

• we have a queryFn that returns a number
• we have a select function that multiplies that number and transforms it to a string.

1. initalData goes through select

you can see this proven in this example:
https://codesandbox.io/s/select-initialdata-3urc4?file=/src/App.tsx

  const { isLoading, error, data } = useQuery(
    "test",
    async () => {
      await waitFor(2000);
      return 5;
    },
    {
      select: (data) => String(data * 10),
      initialData: () => 10
    }
  );

it starts off with printing data is 100, type is string, and then after 2 seconds, it goes to: data is 50, type is string.
That does seem correct to me. initialData is put into the cache and treated like every other data.

However, we have a TypeScript error here: initialData is expected to return a string (TData). That is most certainly not correct, because it goes through select, so it needs to be a number (TQueryData) = the type that the queryFn returns.

I can likely fix that tomorrow, it just means adapting the types.

---

2. placeholderData does not go through select

same example with placeholderData:

https://codesandbox.io/s/select-placeholderdata-38lur?file=/src/App.tsx

  const { isLoading, error, data } = useQuery(
    "test",
    async () => {
      await waitFor(2000);
      return 5;
    },
    {
      select: (data) => String(data * 10),
      placeholderData: () =>10
    }
  );

this prints: data is 10, type is number followed by data is 50, type is string. So clearly, placeholderData does not go through select. Here, we also have the same type-error, but the error is actually correct, given that we don't run through select. the type of data should always be the same (string), otherwise, we can't rely on it. So if we change that to placeholderData: () => String(10), the type error goes away.

Unless of course we want placeholderData to go through select. Then we would need to implement that, and the types would need to change accordingly.

I'm not sure tbh. if we do that, what difference would there be between initialData and placeholderData? According to the docs:

initialData is persisted to the cache, so it is not recommended to provide placeholder, partial or incomplete data to this option and instead use placeholderData

and also:

Placeholder data allows a query to behave as if it already has data, similar to the initialData option, but the data is not persisted to the cache. This comes in handy for situations where you have enough partial (or fake) data to render the query successfully while the actual data is fetched in the background.

So I am not sure if placeholderData should even go through select. select transforms data that comes from the cache, while placeholderData is only shown if we have no data yet and are in loading state.

I'm open to discuss this, and I'd also like some input from @tannerlinsley and @boschni . If we do that, would that be considered a breaking change?

[Svish]

I would love it if at least the types could be corrected to reflect the current behavior, which wouldn't be a breaking change as it's just a correction of types? Probably just a patch bump?

About the greater change, that I don't feel expert enough to weigh in on. Curious to see what the other maintainers think though. 🙃

[TkDodo]

I would love it if at least the types could be corrected to reflect the current behavior, which wouldn't be a breaking change as it's just a correction of types? Probably just a patch bump?

if you’re talking about issue 1) (types of initialData) then yes, that’s a patch I’ll try to tackle tomorrow :)
The types of placeholderData are correct given the current implementation, are they not?

[Svish]

if you’re talking about issue 1) (types of initialData) then yes, that’s a patch I’ll try to tackle tomorrow :)
The types of placeholderData are correct given the current implementation, are they not?

A while since I ran into this now, but I think you have it the right way around, yeah 😛

[Svish]

Thinking about it a bit more, I'm wondering how one would implement my sample code above here though, if placeholderData remains not going through select. Looking at my own code here, it makes sense to me that it should work (except that it currently doesn't).

The point here, is to use placeholder data stored in localStorage, but also go look at the server to make sure it's still correct. I suppose I could also use initialData and immediately invalidate the cache, or something like that, but yeah, not sure what's "best". 🤔

Most of the API of react-query is really great, so wiser minds around here probably has some good ideas/answers.🤓

This is what I hoped would work, but it didn't, because of this issue:

// api.ts
export interface Company { id: number; name: string; }

export function useGetCompanies<R = Company[]>(select?: (data: Company[]) => R): UseQueryResult<R> {
  return useQuery(['api', 'companies'], fetchCompanies, {
    select,
    placeholderData: () =>
      localStorage.getItem<Company[]>('companies') ?? undefined,
    onSuccess: (data) => {
      localStorage.setItem<Company[]>('companies', data);
    },
  });
}

// In some component
const selectOptions = useGetCompanies(
  data => data.map(company => ({ value: company.id, label: company.name }))
);

Works the first time, as the response is stored in localStorage, and the component gets the mapped options via select. But second time around, the component instead gets the original unmapped response stored in localStorage, which of course causes... issues...

Basically, using placeholderData and select together in "wrapper hooks" like this, is currently not really possible. One either needs to not use placeholderData, or not use select. 😕

[tannerlinsley]

I think it would be wise if placeholderData followed the same logical path as initialData and was transformed by select. Does everyone agree? @TkDodo @boschni ?

[TkDodo]

yep, it sounds good 👍

I've created a separate issue (#2060) to tackle the typing problems of initialData in combination with select, because it's not as easy to fix as I thought it would be ;)

[tannerlinsley]

🎉 This issue has been resolved in version 3.15.3 🎉

The release is available on:

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

Your semantic-release bot 📦🚀