Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
HagenMorano opened this issue Aug 13, 2024 · 5 comments
Open
Labels
bug Something isn't working good first issue Straightforward problem, solvable for first-time contributors without deep knowledge of the project openapi-react-query Relevant to openapi-react-query PRs welcome PRs are welcome to solve this issue!

Comments

@HagenMorano
Copy link

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 HagenMorano added bug Something isn't working openapi-fetch Relevant to the openapi-fetch library labels Aug 13, 2024
@drwpow drwpow added the openapi-react-query Relevant to openapi-react-query label Aug 13, 2024
@drwpow
Copy link
Contributor

drwpow commented Aug 13, 2024

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.

@kerwanp
Copy link
Contributor

kerwanp commented Aug 14, 2024

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

Happy to review a PR for that!

@kerwanp kerwanp added PRs welcome PRs are welcome to solve this issue! good first issue Straightforward problem, solvable for first-time contributors without deep knowledge of the project and removed openapi-fetch Relevant to the openapi-fetch library labels Aug 14, 2024
@HagenMorano
Copy link
Author

I'll have a look at it 👍

@HagenMorano
Copy link
Author

HagenMorano commented Aug 14, 2024

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

Options extends Omit<UseQueryOptions<Response["data"], Response["error"]>, "queryKey" | "queryFn">,
(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

@kerwanp
Copy link
Contributor

kerwanp commented Aug 18, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Straightforward problem, solvable for first-time contributors without deep knowledge of the project openapi-react-query Relevant to openapi-react-query PRs welcome PRs are welcome to solve this issue!
Projects
None yet
Development

No branches or pull requests

3 participants