Added generation GraphQLClient.fsi

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

Added generation GraphQLClient.fsi #49

Closed anthony-mi closed 2 years ago

xperiandri commented 3 years ago

@Zaid-Ajaj can I use FusionTasks NuGet to simplify code?

task computation expression does not require Async.AwaitTask, async computation expression requires it. With FusionTasks we can use the same code and simplify generation

Zaid-Ajaj commented 3 years ago

@xperiandri task is already included when asyncReturnType: "task" in which case the Ply package is included

xperiandri commented 3 years ago

I need https://www.nuget.org/packages/FSharp.Control.FusionTasks/2.2.0

Zaid-Ajaj commented 3 years ago

You mean you want to use FusionTasks to simplify the async implementation? I don't really think adding a dependency just for Async.AwaitTask is worth it.

If you need FusionTasks in your application, why not simply add it in your project that references the generated Snowflaqe client?

xperiandri commented 3 years ago

It worse because now we have 2 generator functions everywhere async/task appear which complecates the code a lot and adds more chances for mistake

Zaid-Ajaj commented 3 years ago

I agree it is a bit complicated but it makes the result nicer for users who want Task<'T> as the output of the client function, not just for simplifying the implementation details.

I think implementing error handling option from #29 will be the last hurdle and it's still doable IMO

Do you have any other features might be impacted because of the task/option?

xperiandri commented 3 years ago

I don't know why could people not use https://www.nuget.org/packages/FSharp.Control.FusionTasks/2.2.0 It is one of the first NuGet packages I install to any F# project. Adding Async.AwaitTask to every async .NET BCL call is crazy...

xperiandri commented 3 years ago

The same as manually calling OnPropertyChanged instead of installing PropertyChanged.Fody and make it done automatically

Zaid-Ajaj commented 3 years ago

I don't know why could people not use https://www.nuget.org/packages/FSharp.Control.FusionTasks/2.2.0 It is one of the first NuGet packages I install to any F# project. Adding Async.AwaitTask to every async .NET BCL call is crazy...

Users of Snowflaqe don't have to call Async.AwaitTask, it is just the generated code. You can always install FusionTasks into your own projects whenever you need it.

Sticking with Ply for now as a dependency of the generated projects because it gives a nice task CE to work with when tasks are preferred.

Numpsy commented 3 years ago

Sticking with Ply for now as a dependency of the generated projects because it gives a nice task CE to work with when tasks are preferred.

One of the reasons I made the task change was that 'fits' more when the actual 'async' functionality is using tasks anyway (i.e. HttpClient and the Json libs are task based), but then I was also having a go at inserting the generated lib into an existing C# stack and the tasks returned from Ply just work directly there which is nice for interop.

(I'm a massive beginner with F# and had never heard of FusionTasks, but it looks to me like task { } is going to end up part of FSharp Core at some point, at which point the issue and the requirement for 3rd party libs might just go away? (or maybe i'm being hopeful looking at how long that question has been running for!))

Zaid-Ajaj commented 2 years ago

Hi @anthony-mi what's the status of this PR? Any plans to continue working on?

@xperiandri if there are issues or discussions outside of this PR, please consider opening a separate issue to discuss it there