Skip to content
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

How to contribute genericSeq implementation? #45

Open
njlr opened this issue Oct 21, 2021 · 2 comments
Open

How to contribute genericSeq implementation? #45

njlr opened this issue Oct 21, 2021 · 2 comments

Comments

@njlr
Copy link

njlr commented Oct 21, 2021

Hello!

I think I have figured out how to implement an Auto decoder for seq<'t>:

    open System
    open System.Collections
    open System.Collections.Generic

    type private UnboxedSeq<'t> (inner : obj seq) =
        interface seq<'t> with
          member this.GetEnumerator() : IEnumerator<'t> =
            (seq {
              for x in inner do
                yield x :?> 't
            }).GetEnumerator()

          member this.GetEnumerator() =
            (this :> seq<'t>).GetEnumerator() :> IEnumerator

    let private genericSeq (elementT : Type) (decoder: BoxedDecoder) =
        let unboxedSeqT = typedefof<UnboxedSeq<_>>
        let seqT = unboxedSeqT.MakeGenericType(elementT)

        fun (path : string) (value: JsonValue) ->
            if not (Helpers.isArray value) then
                (path, BadPrimitive ("a seq", value)) |> Error
            else
                let values = value.Value<JArray>()
                (values, Ok []) ||> Seq.foldBack (fun value acc ->
                    match acc with
                    | Error _ -> acc
                    | Ok acc ->
                        match decoder.Decode(path, value) with
                        | Error er -> Error er
                        | Ok result -> result :: acc |> Ok)
                |> Result.map (fun xs -> Activator.CreateInstance(seqT, xs))

However, I'm not sure how to contribute it.

Should I add tests to this repo first?

@MangelMaxime
Copy link
Contributor

Hello,

Yes the tests are hosted on Thoth.Json repository to make sure that both libraries support the same API.

If you need to test locally the test from your fork, you need to publish the forked version of Thoth.Json on github and then update paket.dependencies file to use your fork.

group tests
# Get the tests from Thoth.Json repo so both project are in sync
github thoth-org/Thoth.Json:main tests/Types.fs
github thoth-org/Thoth.Json:main tests/Decoders.fs
github thoth-org/Thoth.Json:main tests/Encoders.fs
github thoth-org/Thoth.Json:main tests/ExtraCoders.fs
github thoth-org/Thoth.Json:main tests/BackAndForth.fs

Also, the generic seq has already been implemented in an unreleased version of Thoth.Json. That specific version of Thoth.Json will never be released because it was too ambitious and I didn't track correctly all the changes I made in it so I am afraid to break people JSON.

I think, it can be a good idea to have a look at it because I think it follows the same API as the current implementation. Using reflection API is always hard to read especially with the need to use boxed/unboxed type to circle around F# limitation.

Here are the places where to find the unreleased implementation:

  1. Place where the auto decoder is registered for the type
  2. Actual auto decoder code

Note: The unreleased implementation use heavily compiler directive because the goal was to unify Thoth.Json and Thoth.Json.Net so you need to pick what code you want to pick depending on the target runtime.

@MangelMaxime
Copy link
Contributor

I tried your solution but it seems to be hurting the same wall I had in the past:

Object of type 'Microsoft.FSharp.Collections.FSharpList1[System.Object]' cannot be converted to type 'System.Collections.Generic.IEnumerable1[System.String]'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants