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.

Sign up for GitHub

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

Jump to bottom

Look into whether we need updates to enable/disable JsonSerializerIsReflectionEnabledByDefault #18057

Closed
rolfbjarne opened this issue Apr 13, 2023 · 3 comments
Closed

Look into whether we need updates to enable/disable JsonSerializerIsReflectionEnabledByDefault #18057

rolfbjarne opened this issue Apr 13, 2023 · 3 comments
Labels
feature A feature to be implemented
Milestone
.NET 8

Comments

@rolfbjarne
Copy link
Member

Refs:

Depending on what happens with these issues, we might have to update something on our end.

So this is just an issue to not forget to do whatever needs to be done once we know if anything actually needs to be done.
@rolfbjarne rolfbjarne added the feature A feature to be implemented label Apr 13, 2023
@rolfbjarne rolfbjarne added this to the .NET 8 milestone Apr 13, 2023
@rolfbjarne rolfbjarne changed the title Look into whether we need up to date to enable/disable JsonSerializerIsReflectionEnabledByDefault Look into whether we need updates to enable/disable JsonSerializerIsReflectionEnabledByDefault Apr 13, 2023
@rolfbjarne
Copy link
Member Author

Looks like we need to set the property to true.

Ref: dotnet/maui#16038
Ref: dotnet/runtime#90395

@rolfbjarne
Copy link
Member Author

We should also add a trivial test:

System.Text.Json.JsonSerializer.Serialize(42)

@rolfbjarne rolfbjarne mentioned this issue Aug 11, 2023
Closed
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Aug 11, 2023
@rolfbjarne
[dotnet] Enable the reflection-based json serializer by default. Fixes …
09d494f 
…xamarin#18057.

Fixes xamarin#18057.
@rolfbjarne rolfbjarne mentioned this issue Aug 11, 2023
Merged
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Aug 11, 2023
@jonathanpeppers
[Xamarin.Android.Build.Tasks] fix System.Text.Json with default trimm…
7469a0a 
…er settings

Fixes: dotnet/maui#16038

Changes to trimmer defaults in .NET 8 require a new
`$(JsonSerializerIsReflectionEnabledByDefault)` MSBuild property to be
set by default for project types that use `$(TrimMode)=partial` like
when running on mobile (iOS/Android).

I added a very basic System.Text.Json test, which fails with the
exception:

    System.InvalidOperationException : JsonSerializerIsReflectionDisabled
    at System.Text.Json.JsonSerializerOptions.ConfigureForJsonSerializer()
    at System.Text.Json.JsonSerializer.GetTypeInfo(JsonSerializerOptions , Type )
    at System.Text.Json.JsonSerializer.Deserialize(String , Type , JsonSerializerOptions )
    at System.Text.JsonTests.JsonSerializerTest.Deserialize()
    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

Simply running the test again with the MSBuild property set allows the
test to pass. This will be a good smoke test for System.Text.Json going
forward.

The iOS side of this is tracked here:

xamarin/xamarin-macios#18057
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Aug 11, 2023
@jonathanpeppers
[Xamarin.Android.Build.Tasks] fix System.Text.Json with default trimm…
658bb80 
…er settings

Fixes: dotnet/maui#16038

Changes to trimmer defaults in .NET 8 require a new
`$(JsonSerializerIsReflectionEnabledByDefault)` MSBuild property to be
set by default for project types that use `$(TrimMode)=partial` like
when running on mobile (iOS/Android).

I added a very basic System.Text.Json test, which fails with the
exception:

    System.InvalidOperationException : JsonSerializerIsReflectionDisabled
    at System.Text.Json.JsonSerializerOptions.ConfigureForJsonSerializer()
    at System.Text.Json.JsonSerializer.GetTypeInfo(JsonSerializerOptions , Type )
    at System.Text.Json.JsonSerializer.Deserialize(String , Type , JsonSerializerOptions )
    at System.Text.JsonTests.JsonSerializerTest.Deserialize()
    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

Simply running the test again with the MSBuild property set allows the
test to pass. This will be a good smoke test for System.Text.Json going
forward.

The iOS side of this is tracked here:

xamarin/xamarin-macios#18057
@jonathanpeppers jonathanpeppers mentioned this issue Aug 11, 2023
Merged
jonathanpeppers added a commit to dotnet/android that referenced this issue Aug 14, 2023
@jonathanpeppers
[Xamarin.Android.Build.Tasks] fix System.Text.Json with default trimm…
05f79c0 
…er settings (#8266)

Fixes: dotnet/maui#16038

Changes to trimmer defaults in .NET 8 require a new
`$(JsonSerializerIsReflectionEnabledByDefault)` MSBuild property to be
set by default for project types that use `$(TrimMode)=partial` like
when running on mobile (iOS/Android).

I added a very basic System.Text.Json test, which fails with the
exception:

    System.InvalidOperationException : JsonSerializerIsReflectionDisabled
    at System.Text.Json.JsonSerializerOptions.ConfigureForJsonSerializer()
    at System.Text.Json.JsonSerializer.GetTypeInfo(JsonSerializerOptions , Type )
    at System.Text.Json.JsonSerializer.Deserialize(String , Type , JsonSerializerOptions )
    at System.Text.JsonTests.JsonSerializerTest.Deserialize()
    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

Simply running the test again with the MSBuild property set allows the
test to pass. This will be a good smoke test for System.Text.Json going
forward.

The iOS side of this is tracked here:

xamarin/xamarin-macios#18057
rolfbjarne added a commit that referenced this issue Aug 15, 2023
@rolfbjarne
[dotnet] Enable the reflection-based json serializer by default. Fixes …
f8b7b53 
…#18057. (#18710)

Fixes #18057.
@rolfbjarne
Copy link
Member Author

Fixed in f8b7b53.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature A feature to be implemented
Projects
None yet
Milestone
.NET 8
Development

No branches or pull requests
1 participant
@rolfbjarne