Fable icon indicating copy to clipboard operation
Fable copied to clipboard

Generate a unique name for anonymous record types

Open MangelMaxime opened this issue 3 years ago • 6 comments

Description

This issue has been reported in https://github.com/thoth-org/Thoth.Json/issues/144

Repro code

open Fable.Core.JsInterop
open FSharp.Reflection
open Fable.Core.DynamicExtensions
open Fable.Core

let inline log x = JS.console.log x

let anonymousValue =
    {|
        A = {| B = "content" |}
    |}

let anonymousType = anonymousValue.GetType()

type RecordB =
    {
        B : string
    }

type RecordA =
    {
        A : RecordB
    }

let realValue =
    {
        A = { B = "content" }
    }

let realType = realValue.GetType()

log (FSharpType.IsRecord(anonymousType, allowAccessToPrivateRepresentation =  true))

let test (typ : System.Type)=
    FSharpType.GetRecordFields(typ, allowAccessToPrivateRepresentation=true)
    |> Array.map (fun fi ->
        let targetKey = fi.Name
        log fi.PropertyType
    )
    |> ignore

test anonymousType // TypeInfo { fullname : '', fields: () => fields }
test realType // TypeInfo { fullname : 'Test.RecordB', fields: () => [[ "B", string_type ]] }

See, how the test anonymousType doesn't have the "B" field information reported.

REPL link

Expected and actual results

We should have access to the nested anonymous record information.

Related information

  • Fable version: 3.7.10

MangelMaxime avatar Jul 28 '22 08:07 MangelMaxime

I need to check how it works in Thoth.Json. In the example above, the reason just seems to be GetRecordFields is not called in the nested record. If I change the last lines I do get the information for the nested record.

let rec printFields (indent: string) (typ : System.Type) =
    for fi in FSharpType.GetRecordFields(typ) do
        let fiType = fi.PropertyType
        printfn $"{indent}{fi.Name}: {fiType.FullName}"
        if FSharpType.IsRecord(fiType) then
            printFields (indent + "    ") fiType

printFields "" anonymousType
printFields "" realType

// OUTPUT:
// A: 
//     B: System.String
// A: Test.RecordB
//     B: System.String

alfonsogarciacaro avatar Jul 30 '22 05:07 alfonsogarciacaro

Ok, I realized the problem is Fable gives an empty name to anonymous records. Although this is an inconsistency with dotnet and maybe we should fix it, for now I sent a PR with a workaround to Thoth.Json: https://github.com/thoth-org/Thoth.Json/pull/145

alfonsogarciacaro avatar Jul 30 '22 06:07 alfonsogarciacaro

@alfonsogarciacaro

Would be nice to have Fable have the same behavior as in .NET, because I have a plan to rewrite Thoth.Json to support both Fable and .NET runtime from the same library.

So if things works differently between both runtime, it can lead to subtle errors. 🙂

Can we please re-open this issue to keep track of this problem?

MangelMaxime avatar Jul 30 '22 12:07 MangelMaxime

The proposed fix will also work in .NET :)

It probably makes sense to give a name to anonymous records with a hash based on field names and types. But please note it won't be exactly the same as .NET (same as type FullNames don't include assembly info in Fable) because .NET F# creates an actual class for anonymous records while Fable just use Pojos.

alfonsogarciacaro avatar Aug 01 '22 01:08 alfonsogarciacaro

The proposed fix will also work in .NET :)

Yes, but there would still be a difference in term of behavior. On Fable, it will not be able to cache the decoder/encoder while on .Net it would cache it because it has a fullname information.

It probably makes sense to give a name to anonymous records with a hash based on field names and types. But please note it won't be exactly the same as .NET (same as type FullNames don't include assembly info in Fable) because .NET F# creates an actual class for anonymous records while Fable just use Pojos.

Not having the same name is totally fine to me :) :+1:

MangelMaxime avatar Aug 01 '22 07:08 MangelMaxime

I think we can fix this in Fable 4. This is because I want to use the hash function we already have for overloads, and in Fable 4 we made a change so you can call it with Fable types. We want to release Fable 4 for JS next month so it's better to wait a bit instead of maintaining a different solution for Fable 3.

alfonsogarciacaro avatar Aug 16 '22 12:08 alfonsogarciacaro