hey-api / openapi-ts

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

Ability to make `OptionsBase`'s `client?: Client` as required field for @hey-api/client-fetch #1290

Open ZilvinasAbr opened 6 days ago

ZilvinasAbr commented 6 days ago

Description

Hi,

Each service function has the options: Options<TData, ThrowOnError>. You can pass the client to it, here's how the interface looks like:

type OptionsBase<ThrowOnError extends boolean> = Omit<RequestOptionsBase<ThrowOnError>, 'url'> & {
    /**
     * You can provide a client instance returned by `createClient()` instead of
     * individual options. This might be also useful if you want to implement a
     * custom client.
     */
    client?: Client;
};

The problem is, is that it is optional: if you don't pass the client, the client that is defined in services.gen.ts is used.

Would it be possible to somehow essentially make it that client is not defined in the services.gen.ts and you would always need to pass the client yourself?

Main reason why we need this: We are using @hey-api/client-fetch in server side. Each user has its own access token, so we need to create a new client for each user server side request. By having the client optional, it is easy to forget to pass the client each time. Forgetting to pass the client would make the applications potentially break, since each user would use the same client object defined in server-side

mrlubos commented 6 days ago

@ZilvinasAbr can you provide more context around your users? Do you never call anything on the client side?

ZilvinasAbr commented 4 days ago

@ZilvinasAbr can you provide more context around your users? Do you never call anything on the client side?

Sure. Essentially, we are using hey-api only on server side and not client side. So when user is calling some of our server side endpoint, from sever side we would need to create the hey-api client each time, since for each user we would need to set different access token.

If used on client side this wouldn't be an issue since each user has their own hey-api client in their browsers, but with server-side this is not the case

ZilvinasAbr commented 1 day ago

@mrlubos I suppose the soon to be released Next.js client would address such issues we are having? Or is that client not intended for server-side Next.js calls?

mrlubos commented 1 day ago

@ZilvinasAbr Next.js client would definitely address issues specific to the Next.js environment(s), including server-side calls. However, expanding clients is not a current priority (the closest to roadmap I have are projects listed in my Sponsors profile)

ZilvinasAbr commented 12 hours ago

@mrlubos I wonder, what added some extra config flag to @hey-api/services that would allow us to for example disable the creation of the client variable inside services.gen.ts file? And make it so that this plugin would generate all the endpoint functions with client as a required prop? I could actually try to help on making such change if that sounds like something valuable to be done.

Or would you see this change as not something valuable for the default @hey-api/services in regards to maintainability/keeping the api of it more light? In that case, I suppose I could try to write some custom plugin that would be mostly 1:1 to the default one, though maintaining it might be tricky.

mrlubos commented 11 hours ago

@ZilvinasAbr Are you able to fork the repository and use it with the desired changes in the meantime? I need to think through how to package this as a feature. So far I've thought about moving the client instantiation into a separate file but not much beyond that to accommodate all these feature requests!