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

[BUG] queryOptions has extra type baggage that's unnecessary with ReactQuery #6395

Closed
davidgoli opened this issue Oct 7, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@davidgoli
Copy link

davidgoli commented Oct 7, 2024

Describe the bug

Look at the types at https://github.com/refinedev/refine/blob/0842b702dad2c95966b2a2b49da0a66527bcbea3/packages/core/src/hooks/show/types.ts#L54C1-L58

  queryOptions?: UseQueryOptions<
    GetOneResponse<TQueryFnData>,
    TError,
    GetOneResponse<TData>
  >;

They should be:

  queryOptions?: UseQueryOptions<
    TQueryFnData,
    TError,
    TData
  >;

(This also applies elsewhere to useTable and other hooks with a queryOptions property)

The problem with the former is they are inconsistent with how ReactQuery defines query types.

For example, if I implement queryOptions.queryFn, It won't accept an API that returns type TQueryFnData, insisting that it's wrapped in a { data: TQueryFnData } container. However, in ReactQuery, you just return TQueryFnData from queryFn and ReactQuery itself owns the metadata/lifecycle container, wrapping the returned value in a QueryObserverBaseResult which contains a { data: TQueryFnData } property.

As a consequence, the result object is doubly wrapped: { data: { data: TQueryFnData }}!

I can work around this by wrapping the response in a redundant GetOneResponse, but this should not be necessary. It would be better if the double wrapping were removed.

Steps To Reproduce

Implement queryOptions.queryFn using options that work in a plain useQuery invocation

Expected behavior

Use of ReactQuery should be a thin wrapper that works along with existing knowledge of the ReactQuery APIs

Packages

@refinedev/core

Additional Context

No response

@davidgoli davidgoli added the bug Something isn't working label Oct 7, 2024
@BatuhanW
Copy link
Member

BatuhanW commented Oct 8, 2024

Hey @davidgoli thanks for taking time to create issue. GetOneResponse is not redundant, it's correct. However, I understand that having double data data may be confusing. This is something we are improving and will remove in the next major version.

@BatuhanW BatuhanW closed this as not planned Won't fix, can't repro, duplicate, stale Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants