Added ability to choose System.Text.Json serializer

[xperiandri]

Looks good! I believe this supersedes #46? First I will get #47 merged in, then we can see how to work with this one

Yes it does

[xperiandri]

@Zaid-Ajaj lets youse string interpolation in code generation. sprintf usage is not readable. OK?

[Zaid-Ajaj]

@Zaid-Ajaj lets youse string interpolation in code generation. sprintf usage is not readable. OK?

Sounds good to me!

[Zaid-Ajaj]

We need to implement a corresponding converter for Fable similar to your converter to allow this scenario. That is why it is not allowed yet. @Zaid-Ajaj, am I wrong?

for serializer = system && target = fable it is not allowed because Fable cannot use System.Text.Json and instead has Fable.SimpleJson

for serializer = system && target = shared it is allowed: shared project stays as is, generated fable project stays as is, only the generated dotnet project will then use System.Text.Json

[xperiandri]

@Zaid-Ajaj, @anthony-mi fixed all comments except the question with Newtonsoft.Json references

[Zaid-Ajaj]

I can take try fix the newtonsoft references issue myself but first, we have to resolve the merge conflict. Can you please rebase on master?

[Zaid-Ajaj]

This look good! thanks @anthony-mi and @xperiandri for the implementation. There is indeed some weird stuff with the newtonsoft references. I will merge this now and try to fix it

[Zaid-Ajaj]

I have just fixed all issues with package references and other small stuff, then published Snowflaqe v1.26 🚀 this version includes the serializer option. Can you please give it a try?

I do have one issue though when serializer = system, the generated client type uses the barebones System.Text.Json without any knowledge about how to deserialize F# types:

type SpotifyGraphqlClient(url: string, httpClient: HttpClient, options: JsonSerializerOptions) =
    new(url: string, options: JsonSerializerOptions) = SpotifyGraphqlClient(url, new HttpClient(), options)
    new(url: string) = SpotifyGraphqlClient(url, new HttpClient(), new JsonSerializerOptions())

Notice here, that by default we use new JsonSerializerOptions() when no options are provided. Shouldn't we include a specialized F# serializer from FSharp.SystemTextJson by default as follows (from docs):

open System.Text.Json
open System.Text.Json.Serialization

let options = JsonSerializerOptions()
options.Converters.Add(JsonFSharpConverter())

I know that consumers of the library could do this themselves but I can imagine it would be better to do it from here so that the deserialization works out of the box without extra configuration. What do you think?

[xperiandri]

Shouldn't we include a specialized F# serializer from FSharp.SystemTextJson by default

Yes, we should. Currently, for Newtonsoft we always insert it to index 0 by default. Maybe we need to align this implementation for both in a fashion where we insert it to index 0 only when it is not present? Or only in the case of default options?

[Zaid-Ajaj]

Only for default options.

To be honest, if we make sure the defaults Just Work out of the box, I believe we don't even need to expose the JsonSerializerOptions to the consumers since they can just start using the client and everything will work like it should 🤞

This is what I did for the default newtonsoft serializer: I removed the options from the constructor of the client because the defaults will work as is.

[Zaid-Ajaj]

Ah one more thing. Using target framework netstandard2.0 was not allowed when packing a dotnet CLI, so I published latest for netcoreapp3.1 and net5.0

[xperiandri]

Not good. You need to publish netcoreapp3.1 only keeping netstandard2.0 present in the library. Because we need netstandard2.0 for MSBuild

[xperiandri]

Only for default options.

OK, we will change that

[Zaid-Ajaj]

To get the netstandard2.0 to work, I think I need to split the tool into two projects: a reusable library targeting netstandard2.0 and a CLI tool targeting netcoreapp3.1/net5.0

[xperiandri]

I suppose you can pass a switch to pack command

[xperiandri]

@Zaid-Ajaj so have you removed the ability to set JsonSerializerSetting for Newtosoft version of a client?

[xperiandri]

My consideration about it is that if you generate your code from the command line you can do whatever you want with sources then. But if you add Snowflaqe from NuGet as MSBuild the code will be built into obj folder and regenerated in project rebuild. So that you will not be able to change anything.

[Zaid-Ajaj]

The thing is, I don't want people to change serialisation settings because what's included is guaranteed to work with these settings. If this is limiting and there is indeed a good reason for changing the settings, I am all ears for it

[xperiandri]

What about custom scalars?

[Zaid-Ajaj]

Custom scalars are converted as string except when they are known custom scalars in which they are converted to specialized types

[xperiandri]

Ah, there is no mechanism to override that yet, right?

[Zaid-Ajaj]

That's correct

[xperiandri]

We need to add that 🙂