-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add JSON null support for the built-in (ReadOnly)Memory converters. #95275
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -20,6 +20,7 @@ internal sealed class JsonMetadataServicesConverter<T> : JsonResumableConverter< | |||||||||||||||||||||
|
||||||||||||||||||||||
internal override Type? KeyType => Converter.KeyType; | ||||||||||||||||||||||
internal override Type? ElementType => Converter.ElementType; | ||||||||||||||||||||||
public override bool HandleNull { get; } | ||||||||||||||||||||||
|
||||||||||||||||||||||
internal override bool ConstructorIsParameterized => Converter.ConstructorIsParameterized; | ||||||||||||||||||||||
internal override bool SupportsCreateObjectDelegate => Converter.SupportsCreateObjectDelegate; | ||||||||||||||||||||||
|
@@ -29,8 +30,16 @@ internal sealed class JsonMetadataServicesConverter<T> : JsonResumableConverter< | |||||||||||||||||||||
|
||||||||||||||||||||||
public JsonMetadataServicesConverter(JsonConverter<T> converter) | ||||||||||||||||||||||
{ | ||||||||||||||||||||||
ConverterStrategy = converter.ConverterStrategy; | ||||||||||||||||||||||
Converter = converter; | ||||||||||||||||||||||
ConverterStrategy = converter.ConverterStrategy; | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change adds some wiring that was being dropped so far. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was about to ask :) Does that mean there's another bug that was lurking and this was fixing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, it was surfaced with the newly added tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For context this is a converter wrapper type that is being used by the source generator APIs. The logic I just added was copied from a similar wrapper type being used internally: Lines 27 to 36 in acf7c85
|
||||||||||||||||||||||
IsInternalConverter = converter.IsInternalConverter; | ||||||||||||||||||||||
IsInternalConverterForNumberType = converter.IsInternalConverterForNumberType; | ||||||||||||||||||||||
CanBePolymorphic = converter.CanBePolymorphic; | ||||||||||||||||||||||
|
||||||||||||||||||||||
// Ensure HandleNull values reflect the exact configuration of the source converter | ||||||||||||||||||||||
HandleNullOnRead = converter.HandleNullOnRead; | ||||||||||||||||||||||
HandleNullOnWrite = converter.HandleNullOnWrite; | ||||||||||||||||||||||
HandleNull = converter.HandleNullOnWrite; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options, scoped ref ReadStack state, out T? value) | ||||||||||||||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.