aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

WebEventData.ParseEventArgsJson has invalid UnconditionalSuppressMessage for JsonSerializer.Deserialize

Open eerhardt opened this issue 3 years ago • 1 comments

See the suppression here:

https://github.com/dotnet/aspnetcore/blob/1d04387cf3b68636f8ade0118334f87ae5f56f7d/src/Components/Web/src/WebEventData/WebEventData.cs#L61-L79

This suppression is not valid because there is no guarantee the eventArgsType hasn't been trimmed, and thus the Deserialization may fail, or behave differently, in a trimmed app.

We should remove this suppression and address the warning correctly so this code behaves the same with and without trimming. Potentially, we can use the same approach that we are taking with https://github.com/dotnet/aspnetcore/issues/45527.

This is the root cause of https://github.com/microsoft/fast-blazor/issues/280. In that issue, the Microsoft.Fast.Components.FluentUI.dll assembly is marked as IsTrimmable=true. The assembly contains some event arg classes, like MenuChangeEventArgs and DialogEventArgs. These classes are being trimmed, since their containing assembly is marked as IsTrimmable=true. However, when an app tries to use these events, it fails as described in the issue. This is because the constructors are being trimmed from the classes, causing the JsonSerializer.Deserialize call above to fail.

cc @brunolins16 @javiercn @JamesNK

eerhardt avatar Jan 03 '23 20:01 eerhardt

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost avatar Jan 05 '23 17:01 ghost

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost avatar Oct 04 '23 17:10 ghost

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost avatar Jan 03 '24 02:01 ghost