FSharp.Json icon indicating copy to clipboard operation
FSharp.Json copied to clipboard

Can not serialize/deserialize type with "internal" or "private" modifier

Open aleksandrkolesnikov opened this issue 5 years ago • 7 comments

Hello. When I try to serialize/deserialize Person, I have an exception

FSharp.Json.JsonSerializationError: 'Failed to serialize, must be one of following types: record, map, array, list, tuple, union. Type is: Person.'

But if I remove internal modifier for Person, the code works fine


type internal Person = {

    [<JsonField("name")>]
    Name: string

    [<JsonField("age")>]
    Age: int
}

[<EntryPoint>]
let main argv =
    let person = { Name = "John"; Age = 42 }
    let json = Json.serialize person
    0

aleksandrkolesnikov avatar Feb 18 '20 08:02 aleksandrkolesnikov

I believe from a dotnet perspective, it's not possible to view type information of a type that is not made public. So it's not possible to view the properties on the object

sanchez avatar Mar 24 '20 04:03 sanchez

@aleksandrkolesnikov What @sanchez wrote is true. Do you have some suggestion on how to retrieve type information for internal or private types?

vsapronov avatar Jun 02 '20 00:06 vsapronov

This can be done by using the correct BindingFlags. See this commit for an example. I'd be happy to help get this implemented.

bzuu-ic avatar Nov 26 '20 13:11 bzuu-ic

While this should not be the primary consideration, private reflection is a lot less efficient. That's one reason why Newtonsoft has the same 'weakness'. (Pretty sure STJ does too). If this was a perf-oriented impl, one might at a minimum expect this to be an opt-in feature at a minimum...

bartelink avatar Nov 26 '20 16:11 bartelink

@bartelink I wasn't aware of the performance implications, but even if the performance would be equal I think this should not be the default behaviour indeed. An overload that allows this would be quite welcome though.

bzuu-ic avatar Nov 26 '20 18:11 bzuu-ic

I think this should not be the default behaviour indeed

I get that; I've been bitten by it too, and it is can be very frustrating, but the fact that this would place this lib on its own in terms of opting people into the cost of private reflection.

Flipping a default like this can be considered a breaking change, depending on how you look at it.

(Its also possible that there's been perf optimization work done to special case the loading such that you only pay extra cost when they are private - that would probably be something that people would be less surprised by, if it were the case...)

bartelink avatar Nov 26 '20 19:11 bartelink

I am mostly thinking along the lines of adding this as a configuration option which can be used with the serializeEx and deserializeEx methods. This allows you to use it only in those circumstances where you actually need it and it makes it a very deliberate choice. For all other cases you can keep the more sensible default.

bzuu-ic avatar Nov 27 '20 08:11 bzuu-ic