UnionContractEncoder - a way to wrap non record fields

[dharmaturtle]

Hi!

FsCodec uses UnionContractEncoder to serialize/deserialize the fields of cases of a DU. For example:

type AddressUpdated = { id: Guid; address: string }

type BasicEventSum =
    | AddressUpdated of AddressUpdated

may be serialized as

{
  "id": "00000005-fff7-fff9-96f7-6f163d79ef1f",
  "address": "123 Main St."
}

This is excellent.

However, a non-record field like the following...

type BasicEventSum =
    | AddressUpdated of Guid

will be serialized to

"00000005-fff7-fff9-96f7-6f163d79ef1f"

This is less excellent because it makes "upgrading" an event to have more fields difficult without resorting to explicit event versioning. One way to fix this is to represent the event as

type BasicEventSum =
    | AddressUpdated of id : Guid

which will be serialized as

{
  "id": "00000005-fff7-fff9-96f7-6f163d79ef1f"
}

You can see how it would simple to upgrade this to the code at the top of this post as it's a purely additive change w/r/t the serialized form.

TypeShape currently does not support this behavior. I see a few ways forward:

1. Add an overload to UnionContractEncoder.Create which may take a custom IMemberVisitor. If this were available, I would be able to inject something similar to the code in this commit.
2. Add an overload with a parameter called "autowrap" (or something) which hardcodes the IMemberVisitor's behavior as demoed in the above commit.
3. Change the requireRecordPayloads parameter to a DU with 3 cases: requireRecordPayloads, wrapNonRecordPayloads, and normal.
4. Copy/paste the Impl module and make the modifications directly in FsCodec. Its maintainer is not fond of this idea for obvious reasons.

Are there any other solutions you think may be better?

Linking the referenced FsCodec issue.

[eiriktsarpalis]

I would recommend that all union case payloads use record types, in order to encourage explicit modelling and evolution of the serialization contract. In fact, earlier versions would throw if the payload used anything but a record type. This has been relaxed by having the encoder generator accept a requireRecordFields argument, however in hindsight the default value should have been true.

Converting values into objects makes implicit assumptions about the encoding format (it is not given that the encoding 'Format supports serializing Map values), so I would not support making such a change. The recommended course of action would be to simply copy and paste the UnionEncoder implementation (which is a standalone file that only consumes public TypeShape APIs) and make adjustments as needed.

[dharmaturtle]

Converting values into objects makes implicit assumptions about the encoding format (it is not given that the encoding 'Format supports serializing Map values), so I would not support making such a change.

This makes a lot of sense to me. Thank you for a very thorough answer, and your comments on my commit.