-
Notifications
You must be signed in to change notification settings - Fork 300
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
ObservableProperty
seems to break JSON Deserialization when used with JsonSerializerContext
#893
Comments
JsonSerializationContext
JsonSerializerContext
JsonSerializerContext
ObservableProperty
seems to break JSON Deserialization when used with JsonSerializerContext
I also encountered this problem |
I also encountered this issue with the following sample class: public partial class Person : ObservableObject
{
[ObservableProperty]
[property: JsonPropertyName("f")]
[NotifyPropertyChangedFor(nameof(FullName))]
string firstName = string.Empty;
[ObservableProperty]
[property: JsonPropertyName("n")]
[NotifyPropertyChangedFor(nameof(FullName))]
string lastName = string.Empty;
public string FullName => $"{FirstName} {LastName}";
} With the application encountering an exception on startup. This appears to be a regression in CommunityToolkit.Mvvm 8.3.0 whereas it is working in 8.2.2. I created a standalone repo https://github.com/stephenquan/Maui.Mvvm.Json.Test that demonstrates this. |
@stephenquan are you able to provide a minimal repro, possibly like a unit test or some self-contained code snippet that replicates the issue? Are you seeing differences in the generated code between 8.2.2 and 8.3.0 in your case? Thank you! |
@Sergio0694 see my comment where I provided a github repo that is a minimal repro. I checked in the 8.2.2 generated file for Person.cs https://github.com/stephenquan/Maui.Mvvm.Json.Test/blob/main/GeneratedFiles/Maui.Mvvm.Json.Test.Person.g.cs.txt and the 8.3.0 generated file is in the mvvm-8.3.0 branch https://github.com/stephenquan/Maui.Mvvm.Json.Test/blob/mvvm-8.3.0/GeneratedFiles/Maui.Mvvm.Json.Test.Person.g.cs.txt You can see the changes between 8.2.2 to 8.3.0 included the GeneratedFiles thru this commit reference: stephenquan/Maui.Mvvm.Json.Test@a8f06ce |
Yeah I saw that, that's why I asked for a minimal repro, as in, a self-contained snippet. If you actually meant that that is a minimal repro, as in, removing MAUI doesn't reproduce the issue anymore, then I suspect this might be unrelated from the MVVM Toolkit itself. I don't even have the MAUI workload installed, that's also why I'm asking for a standalone minimal repro 🙂 |
The generated code is identical (except for |
I don't know why the changing events look different. It definitely wasn't intentional. |
The exceptions in my app are .NET MAUI errors. Below is the exception/stacktrace when the app is in it's startup sequence. I will try creating another app, say, a console app. at System.RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly, Boolean wrapExceptions) |
Oh I see, that's due to #722. By the way, in 8.3.0, you use: <PropertyGroup>
<MvvmToolkitEnableINotifyPropertyChangingSupport>false</MvvmToolkitEnableINotifyPropertyChangingSupport>
</PropertyGroup> If you want to disable support for |
@Sergio0694 I tried the Person class in a straight console top-level application and it works with CommunityToolkit.Maui 8.3.0 with no issue. Would you like me to split my issue into a new one that is "MAUI + CommunityToolkit.Mvvm 8.3.0 + Json" ? Also, because it's MAUI, I'm unsure if my issue belongs in this repo anymore or over in https://github.com/dotnet/maui ? using CommunityToolkit.Mvvm.ComponentModel;
using System.Text.Json.Serialization;
using System.Text.Json;
string json = @"{""f"":""John"",""n"":""Doe""}";
var person = JsonSerializer.Deserialize<Person>(json);
Console.WriteLine(person.FullName); // Output: John Doe
public partial class Person : ObservableObject
{
[ObservableProperty]
[property: JsonPropertyName("f")]
[NotifyPropertyChangedFor(nameof(FullName))]
string firstName = string.Empty;
[ObservableProperty]
[property: JsonPropertyName("n")]
[NotifyPropertyChangedFor(nameof(FullName))]
string lastName = string.Empty;
public string FullName => $"{FirstName} {LastName}";
} |
@stephenquan could you try #923 (comment) in your MAUI app with 8.3.0 and see if that fixes the crash? I wonder if it's the same issue. |
Hi @Sergio0694, thanks for the suggestion, so, I tried adding a WindowsSdkPackageVersion to match the TargetFrameworks .NET MAUI sets, and this is what it looks like: <TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('windows'))">$(TargetFrameworks);net8.0-windows10.0.19041.0</TargetFrameworks>
<WindowsSdkPackageVersion Condition="$([MSBuild]::IsOSPlatform('windows'))">10.0.19041.0</WindowsSdkPackageVersion> But it resulted in the following error: 1>F:\Maui\Maui.Mvvm.Json.Test\Maui.Mvvm.Json.Test\Maui.Mvvm.Json.Test.csproj : error NU1102: Unable to find package Microsoft.Windows.SDK.NET.Ref with version (= 10.0.19041) So I bumped the WindowsSdkPackageVersion up <WindowsSdkPackageVersion Condition="$([MSBuild]::IsOSPlatform('windows'))">10.0.19041.1-preview</WindowsSdkPackageVersion> But this generates a new issue: 1>C:\Program Files\dotnet\sdk\8.0.303\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Windows.targets(55,5): error NETSDK1148: A referenced assembly was compiled using a newer version of Microsoft.Windows.SDK.NET.dll. Please update to a newer .NET SDK in order to reference this assembly. Incidentally, I decided to target my .NET MAUI project for Android, and, surprisingly, the application was built and ran with no issue! So the problem seems to be windows only. |
Also, I note that when my project restore it produces a lot of warnings (note this is before trying your workaround): Rebuild started at 2:22 PM... |
<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('windows'))">$(TargetFrameworks);net8.0-windows10.0.19041.0</TargetFrameworks>
<WindowsSdkPackageVersion Condition="$([MSBuild]::IsOSPlatform('windows'))">10.0.19041.0</WindowsSdkPackageVersion> This is not correct and it's not what I suggested in the comment I linked at all. The package version must end with .41, ie: <TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('windows'))">$(TargetFrameworks);net8.0-windows10.0.19041.0</TargetFrameworks>
<WindowsSdkPackageVersion Condition="$([MSBuild]::IsOSPlatform('windows'))">10.0.19041.41</WindowsSdkPackageVersion> |
Yes, again, like I said in the other issue, this is just an issue with the Windows SDK projections. Just try .41 in your project 🙂 |
Thanks @Sergio0694 , using a package version ending in .41 has resolved the issue for me. As far as I'm concerned, this problem is solved. I don't know if you need to wait for @karmeye to do the same to close this issue. |
I'd like to chime in that I'm encountering this as well, but I think the problem may be with the JSON generator, and not this generator. Here is my class : public partial class SlotGroup {
[ObservableProperty][property: JsonPropertyName(nameof(Name))] protected string name;
[ObservableProperty][property: JsonPropertyName(nameof(Description))] private string description;
[ObservableProperty][property: JsonPropertyName(nameof(ChartTitle))] private string chartTitle;
[ObservableProperty][property: JsonPropertyName(nameof(IsDarkMode))] private bool isDarkMode;
[ObservableProperty][property: JsonPropertyName(nameof(ShowLegend))] private bool showLegend;
[JsonPropertyName("slots")]
public List<SlotGroupItem> Slots { get; init; }
[JsonSerializable(typeof(SlotGroup))]
[JsonSerializable(typeof(SlotGroup[]))]
[JsonSourceGenerationOptions(IncludeFields = false, AllowTrailingCommas = true, WriteIndented = true, GenerationMode = JsonSourceGenerationMode.Metadata)]
public partial class SerializerContext : JsonSerializerContext
{
}
} Here is the generated code for one of the properties -- this looks fine to me: /// <inheritdoc cref="name"/>
[global::System.CodeDom.Compiler.GeneratedCode("CommunityToolkit.Mvvm.SourceGenerators.ObservablePropertyGenerator", "8.2.0.0")]
[global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
[global::System.Text.Json.Serialization.JsonPropertyNameAttribute("Name")]
public string Name
{
get => name;
set
{
if (!global::System.Collections.Generic.EqualityComparer<string>.Default.Equals(name, value))
{
OnNameChanging(value);
OnNameChanging(default, value);
OnPropertyChanging(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangingArgs.Name);
name = value;
OnNameChanged(value);
OnNameChanged(default, value);
OnPropertyChanged(global::CommunityToolkit.Mvvm.ComponentModel.__Internals.__KnownINotifyPropertyChangedArgs.Name);
}
}
} And finally the snippet from the json source generator, which seems to me that it is blind to the attribute that is being applied to the generated property (the only property the json sees is the explicit one) private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] SlotGroupPropInit(global::System.Text.Json.JsonSerializerOptions options)
{
var properties = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[1];
var info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Collections.Generic.List<global::WittmannUsa.OscilloscopeClient.Models.SlotGroupItem>>
{
IsProperty = true,
IsPublic = true,
IsVirtual = false,
DeclaringType = typeof(global::WittmannUsa.OscilloscopeClient.Models.SlotGroup),
Converter = null,
Getter = static obj => ((global::WittmannUsa.OscilloscopeClient.Models.SlotGroup)obj).Slots,
Setter = static (obj, value) => throw new global::System.InvalidOperationException("Setting init-only properties is not supported in source generation mode."),
IgnoreCondition = null,
HasJsonInclude = false,
IsExtensionData = false,
NumberHandling = null,
PropertyName = "Slots",
JsonPropertyName = "slots"
};
properties[0] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Collections.Generic.List<global::WittmannUsa.OscilloscopeClient.Models.SlotGroupItem>>(options, info0);
return properties;
} --- Workaround ---Now, all this being said I did figure out a workaround. The SerializerContext is a public class contained within the parent that it will be serializing. ( so for me its Note that this workaround will not work if referencing another item with [ObservableProperty][JsonInclude] private string name;
[ObservableProperty][JsonInclude] private string description;
[ObservableProperty][JsonInclude] private string chartTitle;
[ObservableProperty][JsonInclude] private bool isDarkMode;
[ObservableProperty][JsonInclude] private bool showLegend;
[JsonPropertyName("slots")]
public List<SlotGroupItem> Slots { get; init; }
[JsonSerializable(typeof(SlotGroup))]
[JsonSerializable(typeof(SlotGroup[]))]
[JsonSourceGenerationOptions(AllowTrailingCommas = true, WriteIndented = true, GenerationMode = JsonSourceGenerationMode.Metadata)]
public partial class SerializerContext : JsonSerializerContext
{
} |
This is also the case when enabling the configuration source generator, e.g, binding a config section to a class that has So it's not just with |
This is a known issue caused by the fact that source generators can't see each other's outputs. |
Describe the bug
Using this, the property is not populated with the json value.
However, if I write it manually it works.
Regression
No response
Steps to reproduce
See description above. Use source-generation based serialization rather than reflection.
Expected behavior
I would expect it to deserialize. Not sure if it's Mvvm issue or System.Text.Json. But reported it here.
Screenshots
No response
IDE and version
VS 2022
IDE version
No response
Nuget packages
Nuget package version(s)
8.2.2
Additional context
Probably serialization also doesn't work.
Help us help you
No, just wanted to report this
The text was updated successfully, but these errors were encountered: