openapi-typescript icon indicating copy to clipboard operation
openapi-typescript copied to clipboard

Wrong type in openapi-react-query when using select to transform data

Open HagenMorano opened this issue 1 year ago • 5 comments

Mind: This is a bug in openapi-react-query. There is no template for opening Bugs for the openapi-react-query package yet.

Description

The select transformer in the queryOptions of the useQuery method expects a value of type data as return value. The value should be transformable to an arbitrary value (and be inherited correctly). Example:

Cool:

$api.useQuery('get', '/pets/person', {},
  {
    select: (data) => data,
  }
);

Not cool:

$api.useQuery('get', '/pets/person', {},
  {
    select: (data) => data.age, // or any other value
  }
);

Reproduction

See the implementation at the bottom of fetchClient.ts in this Stackblitz.

Expected result

Arbitrary return values in the select method do not throw type errors. Furthermore, data should infer the type of the returned value of the select method (if set):

const { data } = $api.useQuery('get', '/pets/person', {},
  {
    select: (data) => data.age, // this shouldn't throw any type errors
  }
);

const str: number | undefined = data; // this shouldn't throw any type errors either as date.age is of type number and optional.

HagenMorano avatar Aug 13 '24 13:08 HagenMorano

Thanks for filing! You’re right—we should add a template for openapi-react-query. Though for now, it’s not an issue opening a bug against openapi-fetch because there’s still overlap in what those 2 libraries do.

drwpow avatar Aug 13 '24 14:08 drwpow

Hi! This is definitely a bug and could be easily fixed by changing the types.

Happy to review a PR for that!

kerwanp avatar Aug 14 '24 08:08 kerwanp

I'll have a look at it :+1:

HagenMorano avatar Aug 14 '24 13:08 HagenMorano

It seems, the only (pragmatic) way to make the return value infer the same value as returned by the select method is to let tanstack query infer the types itself, see also https://tkdodo.eu/blog/react-query-and-type-script#the-four-generics

My understanding is the following:

In the current implementation, the response of useQuery is not inferred via the query options but typed ("manually") in the UseQueryMethod. In theory, there is "only" the TData generic missing in https://github.com/openapi-ts/openapi-typescript/blob/754b31aa6f8ff3d2f5675fd158ddc04d0f53388a/packages/openapi-react-query/src/index.ts#L21 (third parameter of UseQueryOptions). I could not come up with an easy approach to extract TData in the current implementation.

I recently tried to build a wrapper for tanstack query myself and also ran into the problem that the return value of select was not inferred. In my case, I ended up extracting the TData type from the query options which I am passing as a parameter. Code looks something like this

function TanstackQueryWrapper<
  TQueryFnData = unknown,
  TError = DefaultError,
  TData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
>(
  {queryOptions}: {  queryOptions: UseQueryOptions<TQueryFnData, TError, TData, TQueryKey>;}
) {
  const { data } = useQuery(queryOptions) // Data is inferred correctly
}

From my current POV there is more to do here than changing types. Happy to hear more opinions on this and how it could be solved without too much refactoring.

Edit: Maybe also interesting concerning the topic https://twitter.com/TkDodo/status/1491451513264574501

HagenMorano avatar Aug 14 '24 15:08 HagenMorano

Thanks for the detailed answer, this definitely needs to be addressed. I'll try to find some time in the following days to improve the types of the libraries and use more properly the @tanstack/react-query (and hopefully fix this issue).

As you are already advanced on this subject, I am totally open to review a PR that fixes this issue.

kerwanp avatar Aug 18 '24 22:08 kerwanp

@kerwanp opened a MR.

Let's call it a "solution oriented" approach :) I guess leveraging the "native" tanstack query type system would be the cleaner solution but I think this is not possible without making significant changes to the current implementation.

HagenMorano avatar Jan 20 '25 11:01 HagenMorano