Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit types that get converted to null for empty/null input in NullableConverter #60498

Conversation

oroztocil
Copy link
Member

@oroztocil oroztocil commented Feb 19, 2025

Limits the nullable types for which empty string or null input gets converted to null in NullableConverter. After the change this only happens for types that either have TypeCode != TypeCode.Object or belong to a set of special supported types (currently DateOnly, TimeOnly, DateTimeOffset).

Other types get processed by the respective non-nullable converter as was the case before #52499. In particular, types implementing IParsable (with the exception of the types mentioned above) get converted using their TryParse method (via ParsableConverter) which means they can provide custom implementation for handling empty/null inputs.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Feb 19, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 19, 2025
@oroztocil oroztocil force-pushed the fix/55202-limit-nullable-converter-supported-types branch from 5a5a95c to 3dd098b Compare February 19, 2025 17:24
@oroztocil oroztocil changed the title Limit types supported by NullableConverterFactory Limit types that get converted to null for empty/null input in NullableConverter Feb 19, 2025
@oroztocil oroztocil marked this pull request as ready for review February 20, 2025 14:34
@Copilot Copilot bot review requested due to automatic review settings February 20, 2025 14:34
@oroztocil oroztocil requested a review from a team as a code owner February 20, 2025 14:34

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Components/Endpoints/test/FormMapping/Converters/NullableConverterTests.cs:210

  • Consider adding an assertion that verifies result is null when conversion fails, to make the expected behavior more explicit.
var returnValue = nullableConverter.TryConvertValue(ref reader, "bad value", out var result);
@oroztocil oroztocil merged commit 47a6f75 into dotnet:main Feb 20, 2025
27 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview3 milestone Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components 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