-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove JsonValue.fs from client and use System.Text.Json #328
Comments
There are a few things that the homegrown JSON (and the more widely used
type Money =
{
Dollars : int
Cents : int
}
// JSON representation is:
// "123.45"
let decode =
decoder {
let! (x : string) = Decode.string
match x.Split([| '.' |]) with
| [| dollars; cents |] ->
let! dollars =
match Int32.TryParse(dollars) with
| true, i -> Decode.succeed i
| _ -> Decode.fail "Invalid dollars"
let! cents =
match Int32.TryParse(cents) with
| true, i -> Decode.succeed i
| _ -> Decode.fail "Invalid cents"
return { Dollars = dollars; Cents = cents }
| _ ->
return! Decode.fail "Expected <dollars>.<cents>"
}
let json = Decode.unsafeFromString "{ \"foo\": 123 }"
match json with
| JsonValue.Object m ->
match Map.tryFind "foo" m with
| Some (JsonValue.Integer x) -> x
| _ -> 0
| _ -> 0
let a = Encode.object [ "foo", Encode.int 123 ]
let b = Encode.object [ "foo", Encode.int 123 ]
Assert(a = b)
Does If not, perhaps they could be built on-top using active patterns? |
@gusty could you share your opinion? |
I had a glance at Fleece - it looks great! However, would we need to have an
|
I expect to use |
Why specifically |
Do you have any scenario where you need something else? |
I mean, the client shouldn't care about specific Json implementations? I don't know how many times I've had to inject some other Json library or configuration in order to get specific use cases to work. The best would be for it to be pluggable with a default that uses a simple enough interface. |
I don't have the capacity for that. I sponsor part of this library development from my sallery. So if someone wants to present a solution I ready to discuss. But myself I only interested in |
For sure, I know the feeling. There is a limited number of hours I can use to support other open source projects. Since the library uses explicit knowledge that JsonValue has patterns for |
Description
The JsonValue.fs deserializer deserializes
0
intoint
while it must deserialize it into the type requested by the schema. Which isfloat
in my case.We can simply use System.Text.Json package version 5 as it is done in https://github.com/fsprojects/SwaggerProvider/tree/net5 with https://github.com/Tarmil/FSharp.SystemTextJson/
Repro steps
Float
field in GraphQL and return0.0
from itint option
Expected behavior
Value is deserialized into the type defined by the schema
Actual behavior
Value is deserialized into the type chosen by JsonValue.fs logic
Related information
1.0.7
The text was updated successfully, but these errors were encountered: