{"action":"created","issue":{"url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/84","repository_url":"https://api.github.com/repos/MangelMaxime/Thoth","labels_url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/84/labels{/name}","comments_url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/84/comments","events_url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/84/events","html_url":"https://github.com/MangelMaxime/Thoth/pull/84","id":384573365,"node_id":"MDExOlB1bGxSZXF1ZXN0MjMzNzQ1MTQ4","number":84,"title":"Add auto encoders","user":{"login":"alfonsogarciacaro","id":8275461,"node_id":"MDQ6VXNlcjgyNzU0NjE=","avatar_url":"https://avatars0.githubusercontent.com/u/8275461?v=4","gravatar_id":"","url":"https://api.github.com/users/alfonsogarciacaro","html_url":"https://github.com/alfonsogarciacaro","followers_url":"https://api.github.com/users/alfonsogarciacaro/followers","following_url":"https://api.github.com/users/alfonsogarciacaro/following{/other_user}","gists_url":"https://api.github.com/users/alfonsogarciacaro/gists{/gist_id}","starred_url":"https://api.github.com/users/alfonsogarciacaro/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alfonsogarciacaro/subscriptions","organizations_url":"https://api.github.com/users/alfonsogarciacaro/orgs","repos_url":"https://api.github.com/users/alfonsogarciacaro/repos","events_url":"https://api.github.com/users/alfonsogarciacaro/events{/privacy}","received_events_url":"https://api.github.com/users/alfonsogarciacaro/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-11-27T00:53:21Z","updated_at":"2018-12-05T23:32:27Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/MangelMaxime/Thoth/pulls/84","html_url":"https://github.com/MangelMaxime/Thoth/pull/84","diff_url":"https://github.com/MangelMaxime/Thoth/pull/84.diff","patch_url":"https://github.com/MangelMaxime/Thoth/pull/84.patch"},"body":"Fix #75, #68 and #76. Also related to #77.\r\n\r\nThis is finally ready to merge. It would be nice to add more tests, but it'd be great if you (or someone else) could do it, as I need to check other projects. There are many things happening in this PR, so I'll try to summarize them here:\r\n\r\n- Fix #68: Auto encode/decode sets\r\n- Fix #76: Decode.datetime always outputs universal time\r\n- Maps with strings or guids as keys are auto encoded/decoded as json objects (arrays are still accepted)\r\n- Add `Encode.Auto.generateEncoder`. This is more consistent with the decoders and is also used by `Encode.Auto.toString` so auto encoding is more predictable now (the Newtonsoft.Json converters have been removed).\r\n- `Auto.generateEncoder` and `Auto.generateDecoder` now accept an `extra` argument with extra encoders/decoders. This makes it easier to mix auto and manual coders (as discussed [here](https://github.com/MangelMaxime/Thoth/issues/80#issuecomment-441741426)). I've tried to design the API for easy composition but we can still discuss how it should look:\r\n\r\n```fsharp\r\n// Users should cache somewhere the extra coders used in the app\r\nlet extraEncoders =\r\n  Encode.makeExtra()\r\n  |> Encode.withDecimal // I use decimals in the serialized types\r\n  |> Encode.withCustom MyType.Encode\r\n\r\nlet extraDecoders =\r\n  Decode.makeExtra()\r\n  |> Decode.withDecimal // I use decimals in the serialized types\r\n  |> Decode.withCustom MyType.Decoder\r\n\r\n// Then somewhere in the app I generate an encoder\r\nlet encoder = Encode.Auto.generateEncoder<Foo>(extra=extraEncoders)\r\n\r\n// ...or deal directly with the json\r\nDecode.Auto.unsafeFromString<Foo>(json, extra=extraDecoders)\r\n```\r\n\r\n- Because now auto coding is easier to extend, we want to warn the user as soon as possible if there's a missing coder. In order to do so, I'm [failing immediately when a coder is missing](https://github.com/MangelMaxime/Thoth/pull/84/files#diff-fcf19738bdd635ced3cec659bd7097a9R999) so I [disabled these tests](https://github.com/MangelMaxime/Thoth/pull/84/files#diff-0085955556097501404f68ae2f0b3fcdR2173). Is there any reason to wait until actual decoding to fail? This could let some issues go unnoticed.\r\n\r\n- I know you're not convinced about removing support for longs, decimals and bigints from `Auto` ;) but it's not good to penalize all users that don't need these types with a bigger bundle size. As now it's easy to customize `Auto`, users can just cache the extra coders they need once in their app (instead of having a very long file of manual decoders for the whole domain). Also it's tricky to decide which should be the basic types covered by `Auto` (for example, I don't consider `decimal` and `bigint` to be basic types in dotnet either). In fact, we could remove more types from `Auto` and let the users build the extra coders as they need them or even allow other customizations (for example, a different coder for unions).\r\n\r\n@MangelMaxime If you want to merge this, we could release an alpha/beta version so people can give their feedback. Or if you prefer, we can just open a discussion here or in another issue to decide the last details. In any case, I'll try to use this in my project to make sure it's working :)"},"comment":{"url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/comments/444691904","html_url":"https://github.com/MangelMaxime/Thoth/pull/84#issuecomment-444691904","issue_url":"https://api.github.com/repos/MangelMaxime/Thoth/issues/84","id":444691904,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NDY5MTkwNA==","user":{"login":"alfonsogarciacaro","id":8275461,"node_id":"MDQ6VXNlcjgyNzU0NjE=","avatar_url":"https://avatars0.githubusercontent.com/u/8275461?v=4","gravatar_id":"","url":"https://api.github.com/users/alfonsogarciacaro","html_url":"https://github.com/alfonsogarciacaro","followers_url":"https://api.github.com/users/alfonsogarciacaro/followers","following_url":"https://api.github.com/users/alfonsogarciacaro/following{/other_user}","gists_url":"https://api.github.com/users/alfonsogarciacaro/gists{/gist_id}","starred_url":"https://api.github.com/users/alfonsogarciacaro/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alfonsogarciacaro/subscriptions","organizations_url":"https://api.github.com/users/alfonsogarciacaro/orgs","repos_url":"https://api.github.com/users/alfonsogarciacaro/repos","events_url":"https://api.github.com/users/alfonsogarciacaro/events{/privacy}","received_events_url":"https://api.github.com/users/alfonsogarciacaro/received_events","type":"User","site_admin":false},"created_at":"2018-12-05T23:32:27Z","updated_at":"2018-12-05T23:32:27Z","author_association":"CONTRIBUTOR","body":"> About the extraDecoders and extraEncoders, I think we can improve it a bit by forcing the user to provide both encoder and decoder at the same time.\r\n\r\nGood idea! I've followed your suggestion. It complicates a bit the structure (requires two extra files to extract common types) and it's a bit of a breaking change (e.g. `Decoder<'T>` is not in the `Decode` module anymore) but I think it's for the better.\r\n\r\n> The idea, is to make auto decoders fails on a class only if it's not marked as optional. \r\n\r\n[Optional members still return None instead of failing](https://github.com/MangelMaxime/Thoth/pull/84/files#diff-fcf19738bdd635ced3cec659bd7097a9R963). Not entirely sure if this is a good idea, but I'm probably missing the context. What was the issue where this was discussed?\r\n\r\nBTW, I'm already using this in my project and it's working :) We kept a couple of custom decoders and could remove more than 600 lines of code for the rest of the types :tada: Then I have a helper like:\r\n\r\n```fsharp\r\nlet inline decoder<'T> = Decode.Auto.generateDecoder<'T>(extra=extra)\r\n```\r\n"}}
{"actor":{"display_login":"alfonsogarciacaro"}}