Skip to content

Fix UnionConverter.CanConvert for boxed multi-case DU#138

Merged
bartelink merged 6 commits intomasterfrom
copilot/fix-json-serializer-not-supported-exception
Mar 26, 2026
Merged

Fix UnionConverter.CanConvert for boxed multi-case DU#138
bartelink merged 6 commits intomasterfrom
copilot/fix-json-serializer-not-supported-exception

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 26, 2026

When a multi-case F# DU is boxed and serialized via JsonSerializer.Serialize(v :> obj), STJ resolves the runtime type (e.g. Test+Test1 nested type) rather than the union base type. UnionConverter<'T>.CanConvert rejected these case types, so STJ fell through to its built-in UnsupportedTypeConverter.

Resolves #128

Root cause

F# multi-case DUs compile to a base class (Test) with nested subclasses per case (Test+Test1, Test+Test2). STJ looks up [JsonConverter] attributes with inherit: false, so the attribute on the base type is invisible for case types. However, when the converter is registered via options.Converters, STJ does call CanConvert with the case type — which previously returned false.

Single-case and struct DUs are unaffected (no nested types generated).

Changes

  • UnionConverter.CanConvert: Accept types where t.DeclaringType = typeof<'T> (the DU case nested types), and validate isUnion against typeof<'T> instead of t
  • Tests: Three new tests for boxed multi-case DU serialization via options.Converters

Usage

// Register converter via options (required for boxed serialization of multi-case DUs)
let options = JsonSerializerOptions()
options.Converters.Add(UnionConverter<MyUnion>())
JsonSerializer.Serialize(MyCase 42 :> obj, options) // works

// Also works via autoUnionToJsonObject
let options = Options.Create(autoUnionToJsonObject = true)
JsonSerializer.Serialize(MyCase 42 :> obj, options) // works

Known limitation

Attribute-only usage ([<JsonConverter(typeof<UnionConverter<T>>)>] without options.Converters) still fails for boxed multi-case DUs — STJ's inherit: false attribute lookup is not overridable from converter code.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 26, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ bartelink
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

…d serialization

When a multi-case F# DU value is boxed (cast to obj), STJ resolves the
runtime type which is a case-specific nested type (e.g. Test+Test1).
The CanConvert check now also accepts types whose DeclaringType matches
the union type, enabling correct serialization via options.Converters.

Fixes #128

Agent-Logs-Url: https://github.com/jet/FsCodec/sessions/aaf72b53-9296-433f-8717-f71c1dd572d6

Co-authored-by: bartelink <206668+bartelink@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix NotSupportedException in JsonSerializer.Serialize for obj Fix UnionConverter.CanConvert for boxed multi-case DU serialization Mar 26, 2026
Copilot AI requested a review from bartelink March 26, 2026 17:57
@bartelink bartelink marked this pull request as ready for review March 26, 2026 18:17
Copilot AI review requested due to automatic review settings March 26, 2026 18:17
@bartelink
Copy link
Copy Markdown
Collaborator

@copilot please add a terse changelog.md entry citing #138

Agent-Logs-Url: https://github.com/jet/FsCodec/sessions/2967ba7c-bd3b-4039-9164-fcb6cdf79e52

Co-authored-by: bartelink <206668+bartelink@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 26, 2026

@copilot please add a terse changelog.md entry citing #138

Added in 9ca5615.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes System.Text.Json serialization of boxed multi-case F# discriminated unions when UnionConverter<'T> is registered via options.Converters, by allowing the converter to be selected for runtime union case types (nested subclasses like Test+Case1).

Changes:

  • Update UnionConverter<'T>.CanConvert to accept union case nested runtime types when 'T is a multi-case DU base type.
  • Add System.Text.Json-only tests covering boxed multi-case DU serialization through JsonSerializerOptions.Converters.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/FsCodec.SystemTextJson/UnionConverter.fs Expands CanConvert to match boxed multi-case DU runtime types so STJ can select the converter.
tests/FsCodec.NewtonsoftJson.Tests/UnionConverterTests.fs Adds #if SYSTEM_TEXT_JSON tests validating boxed multi-case DU serialization using options.Converters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bartelink bartelink changed the title Fix UnionConverter.CanConvert for boxed multi-case DU serialization Fix UnionConverter.CanConvert for boxed multi-case DU Mar 26, 2026
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@bartelink bartelink merged commit a60f23a into master Mar 26, 2026
@bartelink bartelink deleted the copilot/fix-json-serializer-not-supported-exception branch March 26, 2026 19:11
Copilot AI added a commit that referenced this pull request Mar 27, 2026
…ctory workaround

Agent-Logs-Url: https://github.com/jet/FsCodec/sessions/e652c94e-4947-4dcc-9375-42b68ad34629

Co-authored-by: bartelink <206668+bartelink@users.noreply.github.com>
Copilot AI added a commit that referenced this pull request Mar 27, 2026
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

Successfully merging this pull request may close these issues.

STJ: NotSupportedException for JsonSerializer.Serialize<Object> on DU

4 participants