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

feat(query): generate query return type along with hooks #318

Merged
merged 10 commits into from
Mar 14, 2022

Conversation

CPatchane
Copy link
Contributor

@CPatchane CPatchane commented Feb 11, 2022

Status

READY

Description

When dealing with hooks generated using react-query, it's currently difficult to get the actual return types from the queries (not those from the schemas).
In a case where we process the data from the API (changing the case for example) using a custom client, we can't directly rely on the schemas types that have been generated, we have to get the response type from the queries generated instead.
This PR exported those new types for each functions in order to use them in components that used data from those, looking like:

export type GetDemoQueryResult = NonNullable<AsyncReturnType<typeof getDemo>>
export type GetDemoQueryError = ErrorType<GetDemo400>

// Then we can use like
type MyComponentPropsType = {
  foo: GetDemoQueryResult["foo"]
  bar: GetDemoQueryResult["bar"]
}

Note: We have to use NonNullable for some cases when the client can return undefined (201 requests) and avoid type with | undefined. In that case, the final type will be never.

Wdyt?
I guess it could be considered as a breaking change, should we maybe put this behind a config property and make it optional?

@vercel
Copy link

vercel bot commented Feb 11, 2022

Someone is attempting to deploy a commit to a Personal Account owned by @anymaniax on Vercel.

@anymaniax first needs to authorize it.

@CPatchane CPatchane force-pushed the hook_return_type branch 2 times, most recently from 014a0fb to 249d988 Compare February 11, 2022 14:51
@CPatchane CPatchane changed the title [PROPOSAL] feat(hook): generate query return type for along with hooks [PROPOSAL] feat(query): generate query return type for along with hooks Feb 11, 2022
@melloware
Copy link
Collaborator

Just to clear up my confusion can you post what it generates NOW and what your proposed patch will generate? Or is what you are saying you are generating these "additional" values?

@CPatchane
Copy link
Contributor Author

Sorry for the confusion, indeed this PR generates new types

@anymaniax
Copy link
Owner

Hello @CPatchane, it's a good idea! There is no breaking change from what I see no? You just add new types?

melloware
melloware previously approved these changes Feb 12, 2022
@CPatchane
Copy link
Contributor Author

CPatchane commented Feb 12, 2022

Hello @CPatchane, it's a good idea! There is no breaking change from what I see no? You just add new types?

Indeed, this is only new types for the queries part only.
I was thinking maybe that new types can generate more things some people might not need, but I am clearly ok to add this feature as it and maybe put behind a config according to future feedbacks.

Btw, I just pushed a new change to use MutationResult and MutationError naming suffix for mutations for better consistency.

@CPatchane CPatchane changed the title [PROPOSAL] feat(query): generate query return type for along with hooks [PROPOSAL] feat(query): generate query return type along with hooks Feb 12, 2022
@melloware
Copy link
Collaborator

I personally love this @CPatchane . Since its add new code its really just adding new features and flexibility for users without breaking backwards compatibility I am all for it!

I just started using React Query and I can't believe I did not know about it before but its an absolutely amazing library and this Orval generator makes my life so much easier since I already have OpenAPI generated from my Spring Boot apps!

@CPatchane CPatchane changed the title [PROPOSAL] feat(query): generate query return type along with hooks feat(query): generate query return type along with hooks Feb 12, 2022
anymaniax
anymaniax previously approved these changes Feb 14, 2022
@anymaniax
Copy link
Owner

It's only done for react-query here but we should do it for all the clients

@CPatchane
Copy link
Contributor Author

@anymaniax Is it normal we don't generate hooks for swr mutations? When I generate on my side I only have hook for queries, for mutations I only have functions 🤔

@anymaniax
Copy link
Owner

anymaniax commented Feb 17, 2022

Swr doesn't have mutations like react query

@CPatchane
Copy link
Contributor Author

CPatchane commented Feb 17, 2022

@anymaniax Let me know, it should be good for the other clients :) (swr, angular and axios)

src/types/generator.ts Outdated Show resolved Hide resolved
@CPatchane CPatchane force-pushed the hook_return_type branch 2 times, most recently from c26b7e0 to 31928e6 Compare February 28, 2022 16:21
@CPatchane CPatchane force-pushed the hook_return_type branch 2 times, most recently from bdeb404 to 7fb7d35 Compare March 9, 2022 14:12
@anymaniax
Copy link
Owner

Really great job

@anymaniax anymaniax merged commit d7cb2d4 into anymaniax:master Mar 14, 2022
@CPatchane CPatchane deleted the hook_return_type branch March 14, 2022 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants