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
154 stars 25 forks source link

Implemented support for System.text.Json #50

Closed dim-37 closed 2 years ago

Zaid-Ajaj commented 2 years ago

I just realized this PR adds the FSI files with method signatures only for the constructor. Shouldn't the fsi signature files also emit the generated method names?

xperiandri commented 2 years ago

No, it is not mandatory. Only the members that require modification can be put into fsi

Zaid-Ajaj commented 2 years ago

No, it is not mandatory. Only the members that require modification can be put into fsi

@xperiandri What is then the idea behind adding such a file? Isn't this to improve compiler performance by typing out the definitions beforehand?

@dim-37 Sorry the build is still broken because the Fable build target is using HttpClient (and more non-Fable specific things)

There are still hardcoded client names...

xperiandri commented 2 years ago

What is then the idea behind adding such a file? Isn't this to improve compiler performance by typing out the definitions beforehand?

I cannot add XML comment to default constructor any other way

Zaid-Ajaj commented 2 years ago

I cannot add XML comment to default constructor any other way

All of the code is auto-generated, you could easily add an option for the docs of the client

xperiandri commented 2 years ago

I cannot add XML comment to default constructor any other way

All of the code is auto-generated, you could easily add an option for the docs of the client

I don't understand what you mean

Zaid-Ajaj commented 2 years ago

I mean you can just another configuration option for the xml docs of the client instead of the fsi file

xperiandri commented 2 years ago

In that case, we will need to use only explicit constructors instead of the default one. I thought that adding fsi is simpler 🙂

xperiandri commented 2 years ago

@Zaid-Ajaj do we fixed the remaining comments?

Zaid-Ajaj commented 2 years ago

@anthony-mi @xperiandri I will have another look