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

Export operation types separately #410

Closed
steinathan opened this issue Apr 17, 2024 · 20 comments · Fixed by #500
Closed

Export operation types separately #410

steinathan opened this issue Apr 17, 2024 · 20 comments · Fixed by #500
Labels
feature 🚀 New feature or request

Comments

@steinathan
Copy link

Description

As I highlighted in the screenshot below, can we have the type annotation to be pure type eg IData instead of how it is now? I see it was present in earlier version but not is arbitry and doesn't look nice to be exported and used by other tools like react-query

image

@steinathan steinathan added the feature 🚀 New feature or request label Apr 17, 2024
@steinathan steinathan changed the title Can type annotation not be literals, rather pure types that can be exported Can type annotation not be literals, rather pure types that can be exported & consumed by other libs? Apr 17, 2024
@mrlubos
Copy link
Member

mrlubos commented Apr 17, 2024

Hey @navicstein, got a naming suggestion? Also, can you share how (ugly) it looks when used with react-query?

@steinathan
Copy link
Author

Yes, it used to look something like this

export type TDataUpdateOrgCampaign = {
  campaignId: string;
  requestBody: OrgCampaignUpdateInput;
};

all exported - this is preferred because I could import TDataUpdateOrgCampaign from services, then use it in other places

image

@mrlubos
Copy link
Member

mrlubos commented Apr 17, 2024

I think we can bring this back

@steinathan
Copy link
Author

thanks @mrlubos

or maybe provide it as an option -

I'm currently using it via CLI with taskfile and it could be added as an option

# https://taskfile.dev

version: "3"

dotenv: ['./server/.env']

tasks:
  generate:client:
    cmds:
      - echo "Generating rest-client using endpoint -> $BASE_URL"
      - "cd frontend; npx @hey-api/openapi-ts -i $BASE_URL/openapi.json -o ./src/rest-client  --base $BASE_URL --format --enums javascript --lint --schemas --useOptions"
    silent: false

@mrlubos
Copy link
Member

mrlubos commented Apr 17, 2024

Oh that's sweet. How did you find out about these types? They were both added and modified silently

@steinathan
Copy link
Author

I pay close attention to hey-api/openapi-ts because I'm already using it for one of my side projects on production and it makes API calls much easy, especially when used with react-query

I even had an idea of generating react-query mutations and queries using openapi-ts

@mrlubos
Copy link
Member

mrlubos commented Apr 17, 2024

There's https://github.com/7nohe/openapi-react-query-codegen, what do you think of that? But also, that's the integration we're planning to add next :)

@samvaughton
Copy link

Hey, I am evaluating this library and it is going quite well - this is something that would also be beneficial on my side too 👍

@mrlubos
Copy link
Member

mrlubos commented Apr 17, 2024

Consider it done @samvaughton. I'm only worried about having a naming pattern that's clean and has low chance of clashing. I would hope no one names their parameters TDataxxx but there's always a chance right? Anyway, we will clean this up

@samvaughton
Copy link

Nice one, I've seen comments around the postfix option too, perhaps simple callback options for altering the naming strategies services/types? No need to worry then if there is a clash. Just a thought, it won't affect me personally here :-)

@mrlubos
Copy link
Member

mrlubos commented Apr 17, 2024

That would certainly be an option, but in that case we'd need to provide API for people to detect clashes and handle them. Without that, it wouldn't really be a better solution than a simple postfix

@mrlubos mrlubos changed the title Can type annotation not be literals, rather pure types that can be exported & consumed by other libs? Export operation types separately Apr 19, 2024
@raffi1300
Copy link

Would love this as well - would make the development flow with types in projects much easier!

@raffi1300
Copy link

Another followup thought to this one, for an API endpoint which returns an array, currently the type is something like this:

$OpenApiTs["/example"]["get"]["res"][200]

Which is actually an array type, in my case the return type is:

values: TData[] 
pagination: PaginationDataType

For me to get the type I would then have to do: $OpenApiTs["/example"]["get"]["res"][200]["values"] for example, which is now an array, I would then have to go a level deeper to get the actual data model that I need.

I can then do $OpenApiTs["/example"]["get"]["res"][200]["values"][0] to actually get the type of the model.

It would be great if possible that the actual model was exported, even if they are nested in the return.

If it could be broken down something like below:

$OpenApiTs["/example"]["get"]["res"][200]
export type TDataExampleGetValue = {
  foo: "bar";
};

export type PaginationDataType = {
  count: number;
  total: number;
  offset: number;
};

export type $OpenApiTs["/example"]["get"]["res"][200] = {
  values: TDataExampleGetValue[]
  pagination: PaginationDataType
}

I can now import the data model as a single type value instead of having to get the nested values. Even if they were under the $OpenApiTs, it would still help, if it were $OpenApiTs["modelNameHere"] for example.

@mrlubos
Copy link
Member

mrlubos commented Apr 21, 2024

@raffi1300 It is already exported like that, the only thing this issue is about is exporting top-level service types as individual exports

@rubenfiszel
Copy link

My preference would be for inlining and fully expanding the types up to primitive types in the service methods directly instead of doing a type alias

myMethod({a: string, b: { foo: number }): CancelablePromise<{foo: string}[]>

@mrlubos
Copy link
Member

mrlubos commented Apr 25, 2024

Are you never trying to get the types for parameters or response @rubenfiszel? If yes, how would you do it with this inline approach?

@rubenfiszel
Copy link

@mrlubos in our case we do not but I can see how this could be useful to craft bodies before using them in requests.

Type aliases work well too!

@mrlubos
Copy link
Member

mrlubos commented Apr 27, 2024

Hey all, going to add this in the next release. Data types will be exported as {{ tag }}{{ operation }}Data and response types will be exported as {{ tag }}{{ operation }}Response. Let me know if this works for you once you try it out!

@rubenfiszel
Copy link

Fantastic, thanks a lot!

@mrlubos
Copy link
Member

mrlubos commented May 7, 2024

I guess most of you saw the latest updates on this in v0.43.0. As I haven't heard anything since, I'll assume it works well.

Just noting here that in the upcoming clients, there'll be an option to provide custom config for each request. This means that the function types will change, probably to something like myFunc(options: RequestOptions<TData>). I'm not sure whether this is going to get any bad looks from people, so sharing here in advance to hear any concerns. Also, let me know if anyone wants to try the clients before they're on npm! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🚀 New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants