Generate instance-based SDK for schemas

[RaasAhsan]

Description

Hi! Thank you very much for this project, it has served me extremely well. Today, the hey-api code generator produces an SDK artifact which exports an ordinary JavaScript function for each operation declared in the input OpenAPI schema. This has worked pretty well, but I've run into a two snags:

1. The more pressing one, which is the issue of resolving the underlying client for the SDK to use (FYI I am using fetch). If I understand correctly, according to https://heyapi.dev/openapi-ts/clients/fetch, my two options are to either set a global client, or to specify the client for each call. Our issue with the global client is that we have simulation/test workloads where multiple clients are accessing the API, in the same process space. We use cookies for auth so the underlying fetch instance for each client needs to be distinct. As far specifying the client per call goes, I'd like to avoid it since its really easy to miss one (I believe this behavior was slightly changed in a recent release?)
2. In terms of UX, its difficult to rely on autocomplete for discovery what operations are available in the SDK. This is slightly addressed with wildcard imports, but my editor isn't smart enough to automatically insert one.

A instance-based interface where the client is injected via the constructor would solve both of these problems nicely I think. The client (and other configuration) is set once for the lifetime of that class instance, and you can create multiple of them at will, free of conflicts. Then the client can be passed around as desired to whoever needs it.

Could be implemented as a class or object, but usage could look something like this:

import { createWebSdk } from 'sdk.gen.ts'
const client = createWebSdk({ client: myFetchClient });
const posts = await client.listPosts();

Don't know if this has been considered before or the feasibility of it - but thanks for taking the time to read!

[mrlubos]

Hi, yes! This is in fact one of the most request features and of course will be implemented. The recent change you're referring to is that you can make the client required in each call if you want. Stay tuned for more updates and thank you for using this package!

[marthursson]

The interesting thing here is that the recommended default configuration options for Auth is wildly insecure in a server context. Unless I'm misunderstanding the workings of server-side js, if you do (as recommended in the documentation):

import { client } from 'client/client.gen';

client.setConfig({
  auth: () => '<current_user_token>', 
  baseUrl: 'https://example.com',
});

e.g. in hooks.server.ts/middleware.ts , and then expect that the token of the user making the original request will be used for all calls in the current request scope, you are opening up a pretty nasty security hole, right?

[mrlubos]

@marthursson are you describing a scenario where you'd set the token to X which might belong to user Y and X would be reused by another user Z? If yes, that's definitely an issue. How would you go about avoiding it?

[marthursson]

@mrlubos Exactly, that's my concern.

My suggestion would be to make it possible to supply a client on creation of the DefaultService that can be generated with the sdk plugin (using asClass:true option). Making sure that you use a local service instance (configured with the correct auth token) in the request scope would solve the problem.

Also, I definitely think that the use of client.setConfig should be discouraged, at least in all cases related to auth for server-side use. It should be highlighted in the docs, because it might not be completely obvious for everyone just looking for a nice tool to interact with an OpenApi (and it IS a nice tool, except for this particular detail).

[mrlubos]

Note taken on both points. The docs have been broadly written as "here are the available options and it's up to you to know and consider the tradeoffs" but we could add more guidance. I do want to make them more tailored one day, allowing you to choose your stack and customise the content, e.g. if you're using React, any non-React framework functionality is immediately irrelevant to you. I consider that sort of guidance headed in that direction but definitely a valid point

[chiawendt]

Hi, do we need to implement this with backward compatibility, that is, introduce a new option and keep the old output changed, or can we just override the current asClass: true output in a breaking change?

[mrlubos]

@golopot What's your preference?

[chiawendt]

I think there is little justification for using the old class output, and removing unused code is good for long term codebase modifiability, and reduces config complexity.

[chiawendt]

I think this is solved by #2130

[mrlubos]

@golopot oh you found it. I haven't documented it because it's not fully ready, but did you try it? Works well for you?

[chiawendt]

Yes the produced code is what I wanted. This issue blocked our organization from using the package. Thank you.