hey-api / openapi-ts

🚀 The OpenAPI to TypeScript codegen. Generate clients, SDKs, validators, and more. Support: @mrlubos
https://heyapi.dev
Other
1.34k stars 104 forks source link

Got client #586

Open omridevk opened 6 months ago

omridevk commented 6 months ago

Description

I would like to contribute and add support for Got which is a very popular HTTP lib wrapper of the fetch API. Is this something you would like me to make a PR about? I really want to use this library but all of our code is written using Got and we made a lot of wrappers around it, it would be sad not to be able to use it.

mrlubos commented 6 months ago

Hey @omridevk, let me come back to this once I have the new clients working!

omridevk commented 6 months ago

@mrlubos Yes I started to look at the branch you are working on, is there a plan to when this will be merged?

omridevk commented 6 months ago

and thanks for the quick response :D

mrlubos commented 6 months ago

Oh now I feel shy, didn't know anyone else is watching that branch 😳 it's still missing a few things before I feel comfortable saying it's ready and can move onto other clients:

That's the bare minimum. I'm still thinking whether I want to provide any sort of migration layer or simply say generate a new client and replace the old one. And then some tests, clean up the code, and write documentation. 🚀

omridevk commented 6 months ago

Well if the exposed API changes, ideally a codemod will be awesome.

Oh now I feel shy, didn't know anyone else is watching that branch 😳 it's still missing a few things before I feel comfortable saying it's ready and can move onto other clients:

Yeah I have a habit of looking at other people's source code 🧑‍💻

omridevk commented 6 months ago

@mrlubos Isn't there a way that as part of the config, a consumer can pass a fetch client that confirm to an interface? then you will not need to maintain a large list of clients, you could support the main one but allow the community to add more as time goes on. For example, I do want to add a Ky client as well as Got is mainly used in Node, while Ky is used in browser env.

mrlubos commented 6 months ago

I thought about it. At that point, though, where would that client live? Would everyone write/provide their own clients? If so, it's still easier to provide a shared package @hey-api/client-{name}, no?

omridevk commented 6 months ago

that is non of your concern :) you can provide the most requested one, and the community can create their own packages with clients. as long as it will easy to implement a client, I would be happy to publish a lib with got client for example and a ky-client to npm

omridevk commented 6 months ago

It just not sure you would want to maintain all those clients, if you can offload some of the maintenance to other developers, it is always good :)

mrlubos commented 6 months ago

Yep, have to see if I can come up with a shared API. So far I got only one client (pun intended), it will become more clear after getting Axios working

mrlubos commented 6 months ago

It is my concern btw @omridevk because it impacts discoverability. Those clients should be listed somewhere, I don't want people to write their own clients if there are existing packages out there. The second issue that might come up is updating those clients as you point out. Maintaining them is an extra burden, but at least I could update them faster when needed than rely on 3rd party. If people start using a client and now it doesn't work with the latest openapi-ts version anymore, it prevents them from using the new features and they'll start looking for alternatives. There's a reason openapi-fetch exists, even though openapi-typescript-fetch came before that. I'd much rather host all major clients under single monorepo and people can maintain them, than rely on outside maintainers entirely. Got and Ky have each 10k+ stars and Got has 19m weekly downloads, that should be definitely part of the monorepo.

omridevk commented 6 months ago

totally agree about ky and got, and I agree with your points, as always in programming, there are tradeoffs to any decisions :)

omridevk commented 6 months ago

I do think that providing a way to bring your own client is a really good feature to have

mrlubos commented 6 months ago

Already working on a shared client interface, that's a good call @omridevk 🤝

mrlubos commented 5 months ago

@omridevk I just released the Fetch API client (demo). I imagine Got would look similar to this. Do you want it? 😄

omridevk commented 5 months ago

@mrlubos awesome! I will, at the moment I can't really use this lib as the output doesn't include any file extension which fails when tsconfig is set to moduleResolution: "node16" as reported in my other ticket. I was able to fix it in a fork, but I wasn't able to get the tests working as expected as I wanted to add some tests for this case. Can you maybe assist in this matter? Thanks, appreciate your help

mrlubos commented 5 months ago

Yeah let's do it! How did you implement it?

omridevk commented 5 months ago

@mrlubos Here is the PR: https://github.com/hey-api/openapi-ts/pull/644

omridevk commented 5 months ago

@mrlubos I'll start implementing the Got client today, will share my progress as soon as I have some

mrlubos commented 3 months ago

@omridevk remind me, what did you end up doing with the Got client support?

omridevk commented 3 months ago

I was able to pass got as the fetch implementation

omridevk commented 3 months ago

Will share my code

mrlubos commented 3 months ago

@omridevk Do you think I should still publish a separate client for Got? I'm not sure if people will find it easy to discover this support otherwise. Definitely need to add at least a mention to docs too

omridevk commented 3 months ago

@mrlubos Here's the snippet I use to use Got/Ky as the fetch client.


import type {Got, Request as GotRequest, Response} from 'got'
import type {KyInstance} from 'ky'

export function createFetch(client: Got | KyInstance) {
  return async (request: Request) => {
    const url = isServer ? request.url.toString() : request.url.toString().replace('/ingress', '')
    // this is a thin wrapper to call either Ky or Got
    const result = await client[request.method?.toLowerCase() as 'get'](url, {
      headers: normalizeHeaders(request.headers) as unknown as Headers,
      ...(request.body && {json: await request.json()}),
    })
    return new Response(result.body, {
      headers: result.headers as unknown as Headers,
      status: isGotResponse(result) ? result.statusCode : result.status,
      statusText: isGotResponse(result) ? result.statusMessage : result.statusText,
    })
  }
}

function normalizeHeaders(headers: Request['headers']) {
  const out: GotRequest['headers'] | Headers = {}
  headers.forEach((value, key) => {
    out[key] = value
  })
  return out
}

This way I can use it with SSR and use Got in the server and Ky in the client.

omridevk commented 3 months ago

I think it may be very useful to create a got/ky client, because I think the "fetch-client" has a lot of logic that is not required when using Got, as Got/Ky provide their own hooks and no need for interceptors and other implementation

omridevk commented 3 months ago

My snippet probably has some issues and it doesn't cover all the use cases as I am not copying all the stuff that may be needed from the request, but it is easy to add missing properties, or find a better way of turning native request to Got request

mrlubos commented 3 months ago

Okay, will be a separate package at some point 🤝