-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Move System.Object serialization to ObjectConverter #54436
Conversation
Hello @eiriktsarpalis! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Tagging subscribers to this area: @eiriktsarpalis, @layomia Issue DetailsContinuing the backporting efforts of #54420, this PR brings a change from #54328 that requires review in isolation: Potential breaking changeThe changes introduce a deliberate breaking change when it comes to the handling of
The breaking change concerns the following scenario: using System;
using System.Text.Json;
using System.Text.Json.Serialization;
var options = new JsonSerializerOptions { Converters = { new CustomObjectConverter() } };
string json = JsonSerializer.Serialize(new object(), options);
Console.WriteLine(json);
public class CustomObjectConverter : JsonConverter<object>
{
public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) => writer.WriteNumberValue(42);
public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => throw new NotSupportedException();
} In .NET 5 the above would print
|
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
Show resolved
Hide resolved
...sts/System.Text.Json.Tests/Serialization/CustomConverterTests/CustomConverterTests.Object.cs
Outdated
Show resolved
Hide resolved
…nitial_config * origin/main: (362 commits) [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300) Put Crossgen2 in sync with dotnet#54235 (dotnet#54438) Move System.Object serialization to ObjectConverter (dotnet#54436) Move setting fHasVirtualStaticMethods out of sanity check section (dotnet#54574) [wasm] Compile .bc->.o in parallel, before passing to the linker (dotnet#54053) Change PathInternal.IsCaseSensitive to a constant (dotnet#54340) Make mono_polling_required a public symbol (dotnet#54592) Respect EventSource::IsSupported setting in more codepaths (dotnet#51977) Root ComActivator for hosting (dotnet#54524) Add ILLink annotations to S.D.Common related to DbConnectionStringBuilder (dotnet#54280) Fix finalizer issue with regions (dotnet#54550) Add support for multi-arch install locations (dotnet#53763) Update library testing docs page to reduce confusion (dotnet#54324) [FileStream] handle UNC and device paths (dotnet#54483) Update NetAnalyzers version (dotnet#54511) Added runtime dependency to fix the intermittent test failures (dotnet#54587) Disable failing System.Reflection.Tests.ModuleTests.GetMethods (dotnet#54564) [wasm] Move AOT builds from `runtime-staging` to `runtime` (dotnet#54577) Keep obj node for ArrayIndex. (dotnet#54584) Disable another failing MemoryCache test (dotnet#54578) ...
Continuing the backporting efforts of #54420, this PR brings a change from #54328 that requires review in isolation:
Potential breaking change
The changes introduce a deliberate breaking change when it comes to the handling of
new object()
values. The existing implementation will hardcode the serialization to{}
even if the user has subscribed a customJsonConverter<object>
that does something different. I would like to change this behavior for a few reasons:new object()
values. Users cannot specify a custom serialization format fornew object()
values.object
instance serialization to the relevant converter.new object()
instances are being serialized in production applications.The breaking change concerns the following scenario:
In .NET 5 the above would print
{}
, however under the new changes it would print42
. This could break users with custom object converters who rely on the existing hardcoding behavior.