Skip to content

Fix JsonIgnoreCondition.WhenWritingNull doc - #128127

Open
lilinus wants to merge 2 commits into
dotnet:mainfrom
lilinus:JsonIgnoreCondition.WhenWritingNull-doc
Open

lilinus wants to merge 2 commits into
dotnet:mainfrom
lilinus:JsonIgnoreCondition.WhenWritingNull-doc

Conversation

@lilinus

@lilinus lilinus commented May 13, 2026 •

Copy link
Copy Markdown
Contributor

Doc only

It acutally applies to nullable value types too

using System;
using System.Text.Json;
using System.Text.Json.Serialization;

Forecast forecast = new()
{
    Date = null,
};
JsonSerializerOptions options = new()
{
    DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
};
string forecastJson = JsonSerializer.Serialize<Forecast>(forecast, options);
Console.WriteLine(forecastJson); // Prints "{}"

public class Forecast
{
    public DateTime? Date { get; set; }
};

Should I manually make PR to https://github.com/dotnet/docs ?

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label May 13, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Comment thread src/libraries/System.Text.Json/Common/JsonIgnoreCondition.cs Outdated
@github-actions

github-actions Bot commented Jul 19, 2026 •

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "a28454e1a98e7b1665ff5a43b1145e9152ec887f",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "745263201eb2b63776fc54b08dcffb15423e40f8",
  "last_reviewed_commit": "a28454e1a98e7b1665ff5a43b1145e9152ec887f",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "745263201eb2b63776fc54b08dcffb15423e40f8",
  "last_recorded_worker_run_id": "29675336780",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "a28454e1a98e7b1665ff5a43b1145e9152ec887f",
      "review_id": 4730519683
    }
  ]
}

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Holistic Review

Motivation: The XML documentation for JsonIgnoreCondition.WhenWritingNull stated the condition applies "only to reference-type properties and fields." This is inaccurate: WhenWritingNull also applies to Nullable<T> value-type members, which the PR author demonstrates with a DateTime? repro that serializes to {}.

Approach: A single-line comment change updates the summary text to "This is applied only to reference and nullable value-type properties and fields." No behavioral code is touched.

Summary: The correction accurately reflects runtime behavior. In JsonPropertyInfo, the WhenWritingNull handling is gated on PropertyTypeCanBeNull, which is true for both reference types and Nullable<T>, so null-valued nullable value-type members are indeed ignored under this condition. The wording is clear and grammatically correct. This is a low-risk, documentation-only improvement with no compatibility, performance, or test implications. LGTM.

Note: as the author mentions, equivalent conceptual documentation may also live in dotnet/docs; that is out of scope for this repository change.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 31.1 AIC · ⌖ 20 AIC · ⊞ 10K

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

This branch has not been deployed

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

Labels

area-System.Text.Json community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants