-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add extension method for JsonSerializerOptions.GetTypeInfo #118469
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
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds generic overloads for the GetTypeInfo and TryGetTypeInfo methods in JsonSerializerOptions to provide type-safe alternatives that return JsonTypeInfo<T> instead of the base JsonTypeInfo type. This enhancement improves the developer experience by eliminating the need for manual casting when working with strongly-typed JSON serialization metadata.
Key changes:
- Added
GetTypeInfo<T>()generic method that returnsJsonTypeInfo<T> - Added
TryGetTypeInfo<T>()generic method with strongly-typed output parameter - Updated the reference assembly to include the new method signatures
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs | Implementation of generic GetTypeInfo<T>() and TryGetTypeInfo<T>() methods |
| src/libraries/System.Text.Json/ref/System.Text.Json.cs | Reference assembly updates to expose the new generic method signatures |
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Outdated
Show resolved
Hide resolved
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Outdated
Show resolved
Hide resolved
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Outdated
Show resolved
Hide resolved
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Show resolved
Hide resolved
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Show resolved
Hide resolved
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Outdated
Show resolved
Hide resolved
...braries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but could you also add a couple of rudimentary unit tests?
Co-authored-by: Pranav Senthilnathan <pranav.senthilnathan@live.com>
Co-authored-by: Pranav Senthilnathan <pranav.senthilnathan@live.com>
Co-authored-by: Pranav Senthilnathan <pranav.senthilnathan@live.com>
|
Note; I noticed that there was a duplicate implementation of this pattern in JsonSerializerContext. I'd like to fix both of them together, so I'll prototype this and present an updated API change. |
I don't think that would be strictly necessary for JSC. Source generated instances already expose strongly typed JTOs as static properties. |
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
|
@agocke are there any other changes you were planning to make before putting this PR out of draft? |
|
I need to make sure I caught everything and that all tests pass. |
Fixes #118468