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

use of StringEnum attribute with Decode.Auto and Extra.withCustom #203

Open
joprice opened this issue Jul 11, 2024 · 4 comments
Open

use of StringEnum attribute with Decode.Auto and Extra.withCustom #203

joprice opened this issue Jul 11, 2024 · 4 comments

Comments

@joprice
Copy link

joprice commented Jul 11, 2024

When decoding a union annotated with StringEnum with Decode.Auto.generateDecoder, the generated decoder is simply parsing a string, so it does not fail on unknown enum cases.

Trying to work around this by passing a custom encoder to the extra parameter inadvertently overrides the decoding of all string types, since typeof<X>.FullName for a type with StringEnum is System.String. I have had similar problems with Erase unions, but unsure why.

Although I could avoid StringEnum altogether in most cases, I have some js code I'm interoping with which uses physical equality checks, and currently enum classes create a new instance of the class per instance of the enum, so I'm looking for a way to have a typesafe string wrapper that can have a validating decoder/encoder.

@joprice
Copy link
Author

joprice commented Jul 12, 2024

I came up with a workaround unrelated to this library that I think handles the issues I mentioned. The idea is to define the type in a module with a let for each case, so there is only a instance for each enum case. Then, a custom decoder is created that maps the cases to these values, to avoid the generated decoder creating a unique instance of the class (the use of generateDecoder could also be skipped to avoid the class instantiation overhead). This avoids having to use Erase or StringEnum to get physical equality working.

module State =
    type State =
        | On
        | Off

    let On = On
    let Off = Off

    let decoder: Thoth.Json.Decoder<State> =
        let decoder = Decode.Auto.generateDecoder<State> ()

        decoder
        |> Decode.map (function
            | On -> On
            | Off -> Off)

    let encoder: Thoth.Json.Encoder<State> = Encode.Auto.generateEncoder<State> ()

type Wrapper = { state: State.State }

module Wrapper =
    let extra = Extra.empty |> Extra.withCustom State.encoder State.decoder
    let decoder = Decode.Auto.generateDecoder<Wrapper> (extra = extra)
    let encoder = Encode.Auto.generateEncoder<Wrapper> ()

https://github.com/joprice/fable-repro/blob/bb83936c10d1ffe9a6d577980aa554d109a10572/Program.fs

@MangelMaxime
Copy link
Contributor

I don't think there is something we can improve inside of Thoth.Json to checks the validity of the provided string for a StringEnum. We just don't have the information available at runtime anymore because the type is erased.

Trying to work around this by passing a custom encoder to the extra parameter inadvertently overrides the decoding of all string types, since typeof<X>.FullName for a type with StringEnum is System.String. I have had similar problems with Erase unions, but unsure why.

I believe this is expected because the types are erased so their representation at runtime is their underlying type. I don't know if this is possible to have the reflection APIs represents the types at compilation time instead of the runtime types and if this would breaks something or not in Fable.

If this needs to be explore more, an issue should be opened on Fable repository to ask helps from others maintainer.


You could perhaps use the cached version of the decoder/encoder if you want to squeeze a bit more performance out:

https://thoth-org.github.io/Thoth.Json/documentation/auto/caching.html


@joprice
Copy link
Author

joprice commented Jul 13, 2024

That makes sense. This may be considered a bug if you compare the behavior of this code when run through fable and fsc

type X = A | B | C

module X = 
  let A = A

let eq1 = X.A = X.A
let eq2 = LanguagePrimitives.PhysicalEquality X.A X.A
let eq3 = LanguagePrimitives.PhysicalEquality X.B X.B

// fable prints true true false 
// fsc prints true true true 
printfn "%b %b %b" eq1 eq2 eq3

@joprice
Copy link
Author

joprice commented Jul 13, 2024

I opened an issue on the fable repo. Perhaps this repo should have a warning in the readme about the type-unsafety of using Thoth with those annotations.

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