Zaid-Ajaj / Snowflaqe

A dotnet CLI to generate type-safe GraphQL clients for F# and Fable with automatic deserialization, static query verification and type checking
MIT License
157 stars 26 forks source link

Create an F# client using just a HttpClient / Interop with HttpClientFactory #38

Closed Numpsy closed 2 years ago

Numpsy commented 3 years ago

Hi,

I've only had a brief go with Snowflaqe so far and might be jumping in a bit quickly, but in case this counts a an interesting or reasonable thought:

I have an existing REST API using ASPCore and NSwag/OpenApi, and am having a play with GraphQl to see if it can simplify some things. The consumers of that are using NSwag to generate C# client classes, and use HttpClientFactory with typed clients to generate class instances (that allows centralised configuration of HttpClient settings, plus supports wiring the clients into Polly and the host logging setup). The NSwag client generator can generate classes with several different approaches of providing HttpClients and the server base URL, and we have it set up so that the generated classes only get given a pre-configured HttpClient and not the URL (which is nice when using DI as injecting plain strings into classes doesn't really work).

I had a go at wiring the Snowflaqe generated clients into the same DI setup and it doesn't work on its own because of the constructors taking strings (and/or having multiple constructors available). This isn't a major problem because I can always register things in the DI container using factory delegates (and manually create a client class instance from a HttpClient I make myself and a null URL), but I wonder if it would be considered reasonable or useful to be able to configure it to generate clients with a single constructor that takes a HttpClient, and nothing else?

i.e., this is a wordy and roundabout request for the ability to generate clients whose constructor is just

type %s(httpClient: HttpClient) =

:-)

Zaid-Ajaj commented 3 years ago

Hi @Numpsy this sounds like a nice addition to the generated client. It is a minor issue but I guess another constructor won't hurt and we can set the url to use the base url of the input client. Will be happy to accept PRs for this one šŸ˜‰

xperiandri commented 3 years ago

@Numpsy do you mean passing HttpClient with BaseAddress set to GraphQL endpoint?

Numpsy commented 3 years ago

Yes. As it stands now you can pass a null URL string to the graphql client constructor and that seems to work ok (though not necessarily obvious from the outside that that is allowed?), and you can wire it in to the DI infrastructure in ASP.Net Core and such using IHttpclientFactory by doing something like:

    services.AddScoped(sp =>
    {    
        var httpClient = sp.GetRequiredService<IHttpClientFactory>().CreateClient("graphql");
        return new sampleGraphqlClient(null, httpClient);
    });

A additional constructor that just takes a HttpClient might avoid the null there, but I don't think you can use the typed HttpclientFactory functions such as

services.AddHttpClient<sampleGraphqlClient>()

unless that is the only constructor (because it can't resolve which constructor to use, and string params don';t get on with the DI setup either). However, I don't know if it's worth giving the client generator the ability to generate client classes with a single constructor just for that, given the simple work around.

xperiandri commented 3 years ago

@Zaid-Ajaj I think it is a good idea. We can add this into the latest PR

Zaid-Ajaj commented 3 years ago

Adding another constructor accepting only HttpClient sounds good, since it is not a breaking change. Then we can for example use base url / base address to initialize the url parameter. No need for another config option to implement this āœ”ļø

Zaid-Ajaj commented 3 years ago

@Numpsy I do wonder though, when you say:

services.AddSingleton<SampleGraphqlClient>()

Where exactly do you provide the endpoint of the GraphQL API if a generic HttpClient is resolved from there?

xperiandri commented 3 years ago
services.AddHttpClient<SampleGraphqlClient>((provider, httpClient) =>
{
    var configuration = provider.GetRequiredService<IConfiguration>();
    var endpointUrl = configuration["GraphQLEndpoint"];
    httpClient.BaseAddress = endpointUrl;
})
xperiandri commented 3 years ago

@Zaid-Ajaj I think it is a good idea. We can add this into the latest PR

Adding another constructor accepting only HttpClient sounds good, since it is not a breaking change.

@anthony-mi

Numpsy commented 3 years ago
services.AddHttpClient<SampleGraphqlClient>((provider, httpClient) =>
{
    var configuration = provider.GetRequiredService<IConfiguration>();
    var endpointUrl = configuration["GraphQLEndpoint"];
    httpClient.BaseAddress = endpointUrl;
})

That's basically it, though i'm currently using the named client version together with services.AddScoped, e.g.

services.AddHttpClient("sampleGraphqlClient", (provider, httpClient) =>
{
    ...
})

services.AddScoped(sp =>
 {    
     var httpClient = sp.GetRequiredService<IHttpClientFactory>().CreateClient("sampleGraphqlClient");
     return new sampleGraphqlClient(null, httpClient);
 });
Zaid-Ajaj commented 3 years ago

@Numpsy if you want to avoid passing null, wouldn't you just do this:

services.AddScoped(sp =>
{    
     var httpClient = sp.GetRequiredService<IHttpClientFactory>().CreateClient("sampleGraphqlClient");
     return new SampleGraphqlClient(httpClient.BaseAddress, httpClient);
});

šŸ¤”

xperiandri commented 3 years ago

@Zaid-Ajaj can we substitute headers with HttpClient parameter? Will not there be to many constructors if we leave headers?

Zaid-Ajaj commented 3 years ago

What headers?

xperiandri commented 3 years ago

You have 2 parameters for Newtonsoft client:

I propose:

Zaid-Ajaj commented 3 years ago

The first combo of Client(url: string) and Client(url: string, headers: Header list) is only for Fable targets which don't have HttpClient, so to customize the underlying requests, you can only modify the headers. The Header type from the constructor is coming from Fable.SimpleHttp (again, Fable only packages)

The second combo is for fsharp targets where users can access and provide HttpClient

xperiandri commented 3 years ago

Ah, got it

Zaid-Ajaj commented 2 years ago

This feature was added since v1.30 (currently on v1.31)