Skip to content

With ReferenceHandler.IgnoreCycles, registering the factory changes serialized output when a cycle passes through an unclaimed (polymorphic) ancestor #90

Open

Description

@matt-edmondson

What's wrong

JsonRequiredConditionallyConverter.Write (JsonRequiredConditionally/JsonRequiredConditionallyConverter.cs:104-105) calls JsonSerializer.Serialize(writer, value, plainOptions). That starts a fresh serialization with its own reference/cycle tracker.

SerializerFeatureGuard.cs:113-115 and the README both present IgnoreCycles as supported, writing null where a cycle would close.

Repro

All runs use options with ReferenceHandler = ReferenceHandler.IgnoreCycles.

public class Node
{
    public string? Kind { get; set; }
    [JsonRequiredIfSiblingIs(nameof(Kind), "A")] public string? Tuning { get; set; }
    public List<Node>? Children { get; set; }
    public PolyBase? Owner { get; set; }
}
[JsonDerivedType(typeof(PolyDerived), "d")] public class PolyBase { public Node? N { get; set; } }
public class PolyDerived : PolyBase { }

var p = new PolyBase();
p.N = new Node { Kind = "B", Owner = p };
JsonSerializer.Serialize(p, options);

Polymorphic ancestor

Output
Without the factory {"N":{"Kind":"B","Tuning":null,"Children":null,"Owner":null}}
With the factory {"N":{"Kind":"B","Tuning":null,"Children":null,"Owner":{"N":null}}}

Root list. Here l = [a] and a.Children = l.

Output
Without the factory [{…"Children":null…}]
With the factory [{…"Children":[null]…}]

I reproduced both on net10.0 against HEAD b518593. PR #87 will probably fix the root-list case. The polymorphic case remains.

Why it matters

Registering a validation library should not change the JSON it writes. Here it silently adds a level of nesting and moves where the cycle is cut. Anything that compares, hashes, or round-trips the output sees different results depending on whether the factory is registered. The existing test ContainmentTests.IgnoreCyclesSerializesACyclicGraphThroughAClaimedType (ContainmentTests.cs:119) only covers a cycle rooted at the claimed type.

Suggested fix / acceptance criteria

One of:

  • Write in a way that keeps the outer write stack's cycle detection, for example by serializing with the caller's options and a JsonTypeInfo obtained from plainOptions. Whether System.Text.Json allows this needs checking. Or:
  • Document in the README's IgnoreCycles paragraph that a cycle crossing an unclaimed ancestor is cut one level later.

Tests: either the polymorphic repro above serializes the same with and without the factory, or the README states the limitation.

Activity

  1. matt-edmondson commented on Oct 29, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions