Add auto encoder/decoder #30

alfonsogarciacaro · 2018-05-31T09:30:58Z

This still needs some work but it's looking good :) Please have a try and let me know what you think, the new API and function names are just tentative (though we need a static member for optional arguments). I only implemented it in Fable side for now, but a similar approach should work for .NET too, though we will likely have issues with generic casting (unbox< Decoder<obj> > int). We will need to add an untyped interface (like IDecoder) or use something like the crates they talked about in latest F# eXchange.

I also took chance to remove some of the JS code (either in its own file or in Emit attributes) and replace it with F# code, to make it more maintainable. I hope that's OK.

Besides that, I replaced rollup because I haven't updated the plugin yet for Fable 2. And I had some issues with the Thoth.Json.Net.fsproj reference when compiling the tests for Fable so I added an MSBuild condition. But now that I think about it, maybe it's because I used the allFiles option for fable-splitter and removing it may just be enough, hmmm 🤔

alfonsogarciacaro · 2018-05-31T09:50:08Z

Ok, at the end I removed the allFiles option. It looks cleaner that way :)

MangelMaxime · 2018-05-31T11:46:08Z

I used rollup because I needed to read the generate JavaScript when creating the library. And webpack doesn't really fit that need :)

Now, that most of the lib has been written it should be ok to use webpack instead.

MangelMaxime · 2018-05-31T11:54:32Z

I wonder if the new implementation of Encode.list will fix #22

And I do prefer the new implementation without the JavaScript files. 👍

MangelMaxime · 2018-05-31T20:26:15Z

We should add an option to specify if we want the json to use camelCase or PascalCase properties. This is important when working with another language than F# on the server side.

Also, is the Fable 2 reflection API allowing us to add attributes to record properties to do something like:

type R = { [<PropName("foo")>] Bar of string }

alfonsogarciacaro · 2018-05-31T23:36:36Z

I used rollup because I needed to read the generate JavaScript when creating the library. And webpack doesn't really fit that need :)

Indeed. I actually replaced Rollup with fable-splitter for the tests, so the code should still be readable (and also in multiple files).

I wonder if the new implementation of Encode.list will fix #22

I added a test for nested lists. Seems to work :)

We should add an option to specify if we want the json to use camelCase or PascalCase properties.

I'll leave that as an exercise for you and other Thoth.Json contributors ;)

Also, is the Fable 2 reflection API allowing us to add attributes to record properties?

Nope, there's very little reflection info made available (only the Type fullname and generics, besides field and case info only for records and generics respectively) and attribute info cannot be accessed at runtime (you could use CompiledName but I wouldn't recommend it for this). As I see it, reflection should be used by devs which just want a quick a simple serializer. Those who need more fine-tuning can still write their own decoder. Using attributes would defeat a bit Thot's purpose, as serialization metadata would become tightly coupled with the type.

Anyways, the good news is I could enable automatic (de)serialization for .NET too! Unfortunately I couldn't follow the same approach as I was getting runtime errors when trying to cast values parsed with Decoder<obj> to their proper type. So at the end I had to use Newtonsoft.Json converters (as in Fable 1). I don't like it very much, as it's verbose, error-prone, difficult to test and maintain (especially because the code now is much different with Thoth.Json for Fable), but let see how it goes.

MangelMaxime · 2018-06-01T06:20:57Z

src/Thoth.Json.Net/Decode.fs

@@ -460,3  476,11 @@ let optional key valDecoder fallback decoder =

 let optionalAt path valDecoder fallback decoder =
    custom (optionalDecoder (at path value) valDecoder fallback) decoder
+
+type Auto =


@alfonsogarciacaro Can you please explain me what this type do ?

It's necessary to have a type in order to use static members for optional arguments (in Thoth.Json, ITypeResolver). Here I'm just using it to have the same API surface as Thoth.Json, but I guess a module should also work (we don't need the injectable optional argument on the .NET side).

MangelMaxime · 2018-06-01T08:32:38Z

I'll leave that as an exercise for you and other Thoth.Json contributors ;)

I will do it so I can better understand your code :)

Anyways, the good news is I could enable automatic (de)serialization for .NET too! Unfortunately I couldn't follow the same approach as I was getting runtime errors when trying to cast values parsed with Decoder to their proper type. So at the end I had to use Newtonsoft.Json converters (as in Fable 1). I don't like it very much, as it's verbose, error-prone, difficult to test and maintain (especially because the code now is much different with Thoth.Json for Fable), but let see how it goes.

That's unfortunate, we will need to make it clear that auto mode on .Net side will not validate the data.

Is this runtime error limitation due to how .Net works ?

alfonsogarciacaro · 2018-06-01T12:41:49Z

Ah, I didn't think of that! Well, Newtonsoft.Json error reporting is quite good: you get the path and the location (line and column) of the issue in the JSON string. Though it's true we can improve some of the error messages.

Fable erases generics at runtime, so it's possible to do a cast from obj list to int list but this fails in .NET. I think it's still possible to create individual decoders with more reflection magic but it would be more work. Maybe we can try some corrupted JSON to see if the error messages are good enough.

alfonsogarciacaro · 2018-06-05T08:08:12Z

@MangelMaxime We probably should add more tests and improve the error messages in the .Net converters, but for now I think this is ready to merge. I tested it with one of my work projects and it's working 🎉 so it'll be nice to have an alpha release of Thoth.Json compatible with Fable 2.

MangelMaxime · 2018-06-05T13:12:21Z

@alfonsogarciacaro I removed static member GenerateDecoder(t: System.Type): Decoder<obj> = from Thoth.Json.Decode to keep the same API between both Fable & .Net version.

But perhaps we can add the missing one to .Net version. Not sure how to do that however

alfonsogarciacaro · 2018-06-05T13:33:39Z

src/Thoth.Json/Decode.fs

@@ -543,12  543,21 @@ let private mixedArray msg (decoders: Decoder<obj>[]) (values: obj[]): Result<ob
            | Error _ -> acc
            | Ok result -> decoder value |> Result.map (fun v -> v::result))

-let rec private autoDecodeRecordsAndUnions (t: System.Type): Decoder<obj> =
+let inline private lowerCase (s: string) =
+    s |> Seq.mapi (fun i c -> match i with | 0 -> (System.Char.ToLower c) | _ -> c)  |> System.String.Concat


Nice work! One suggestion though, this is traversing through all the chars just to lower the first one, slicing fits here nicely: (string s.[0]).ToLower() s.[1..]. I'd also give more information in the function name, even if it's private: lowerFirstChar

Or also: s.[..0].ToLowerInvariant() s.[1..]

Yes I saw this problem about traversing all the chars and didn't know how to fix it :)

Will update using slicing.

alfonsogarciacaro · 2018-07-11T11:42:59Z

Closing as this has been superseded by #38

MangelMaxime reviewed Jun 1, 2018

View reviewed changes

alfonsogarciacaro mentioned this pull request Jun 1, 2018

Serializing int64 inside discriminated unions generates irreversable JSON representation fable-compiler/Fable#1411

Closed

alfonsogarciacaro changed the title ~~[WIP] Add auto encoder/decoder~~ Add auto encoder/decoder Jun 5, 2018

alfonsogarciacaro mentioned this pull request Jun 5, 2018

JSON deserialization of postRecord response of a DU does not work as expected fable-compiler/fable-powerpack#51

Closed

alfonsogarciacaro commented Jun 5, 2018

View reviewed changes

alfonsogarciacaro and others added 16 commits June 7, 2018 10:20

Update dependencies

d4fbbe0

Simplify JS interop

c9084af

Add auto decoder

69c0aec

Remove allFiles option in splitter config

52069ef

Better reflection

da121e6

Add test for nested lists

6be5b29

Add auto encode/decode to Json.Net

bdacb30

Uniform APIs

de99653

Update Fable and other improvements

839e6ef

Update to Fable 2.0.0-alpha-001 and uniform API

c63ab28

Fix decoding of recursive types

91d3d13

Automatically cache Newtonsoft.json converters

f53ca7a

Moar fixes

5b385a1

Add camelCase support for Decode.Auto

978e855

Add camelCase support on Thoth.json.Net.Decode

7403c42

Use slicing instead of Seq.map for lowercase

3195901

Maxime Mangel and others added 5 commits June 7, 2018 11:00

Fix yarn.lock after rebase

03d7356

Fix issues

24d95ad

Add some decoders

c8d606c

Update Fable

455bf3f

Use decoders also in Thoth.Json.Net.Decode

41b15cd

alfonsogarciacaro mentioned this pull request Jun 19, 2018

Change Decode.option implementation #33

Closed

MangelMaxime mentioned this pull request Jun 21, 2018

Version 2 of Thoth.Json #36

Closed

9 tasks

alfonsogarciacaro closed this Jul 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add auto encoder/decoder #30

Add auto encoder/decoder #30

alfonsogarciacaro commented May 31, 2018

alfonsogarciacaro commented May 31, 2018

MangelMaxime commented May 31, 2018 •

edited

Loading

MangelMaxime commented May 31, 2018 •

edited

Loading

MangelMaxime commented May 31, 2018

alfonsogarciacaro commented May 31, 2018

MangelMaxime Jun 1, 2018

alfonsogarciacaro Jun 1, 2018

MangelMaxime commented Jun 1, 2018

alfonsogarciacaro commented Jun 1, 2018

alfonsogarciacaro commented Jun 5, 2018

MangelMaxime commented Jun 5, 2018

alfonsogarciacaro Jun 5, 2018 •

edited

Loading

alfonsogarciacaro Jun 5, 2018 •

edited

Loading

MangelMaxime Jun 5, 2018

alfonsogarciacaro commented Jul 11, 2018

Add auto encoder/decoder #30

Add auto encoder/decoder #30

Conversation

alfonsogarciacaro commented May 31, 2018

alfonsogarciacaro commented May 31, 2018

MangelMaxime commented May 31, 2018 • edited Loading

MangelMaxime commented May 31, 2018 • edited Loading

MangelMaxime commented May 31, 2018

alfonsogarciacaro commented May 31, 2018

MangelMaxime Jun 1, 2018

Choose a reason for hiding this comment

alfonsogarciacaro Jun 1, 2018

Choose a reason for hiding this comment

MangelMaxime commented Jun 1, 2018

alfonsogarciacaro commented Jun 1, 2018

alfonsogarciacaro commented Jun 5, 2018

MangelMaxime commented Jun 5, 2018

alfonsogarciacaro Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

alfonsogarciacaro Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

MangelMaxime Jun 5, 2018

Choose a reason for hiding this comment

alfonsogarciacaro commented Jul 11, 2018

MangelMaxime commented May 31, 2018 •

edited

Loading

MangelMaxime commented May 31, 2018 •

edited

Loading

alfonsogarciacaro Jun 5, 2018 •

edited

Loading

alfonsogarciacaro Jun 5, 2018 •

edited

Loading