bgavrilMS
commented
2 years ago Excellent bug. Just so happens that we are also exploring this area, as we want to make MSAL compile with NET7's AOT compiler. Here's are the notes that @pmaytak wrote on the subject.
Problem statement:
- Reflection-based code does not work well with Ahead-Of-Time (AOT) compliers. As a workaround, usually a whole assembly that uses reflection needs to be added into the package increasing final package size.
- Estimate the work of moving JSON serialization to not use reflection in MSAL.
Possible solutions:
- Update current Newtonsoft JSON.NET implementation via flags to disable reflection as was done for Unity. a.
- For Unity related work, build flags were added to exclude reflection code in JSON.NET code for .NET standard and some .NET Fx targets.
- The changes were based on this AOT fork of JSON.NET - jilleJr/Newtonsoft.Json-for-Unity: Newtonsoft.Json (Json.NET) 10.0.3, 11.0.2, 12.0.3, & 13.0.1 for Unity IL2CPP builds, available via Unity Package Manager (github.com).
- This work would include updating our JSON.NET code to probably match the AOT fork. If it's not clear what specifically needs to be changed, then may require running MSAL and testing where it breaks.
- Would also have to test specifically on targets which will be compiled for AOT.
- ETA can be as short as a day, if what was done for Unity already works or minimal flags need to be added.
-
Add System.Text.Json package for supported targets, while keeping JSON.NET for older targets ? a. First need to research System.Text.Json feature and framework availability. i. Source generation is only for .NET 5+. How to choose reflection or source generation in System.Text.Json | Microsoft Docs/ ii. Differences with JSON.NET need to be verified. Migrate from Newtonsoft.Json to System.Text.Json - .NET | Microsoft Docs b. Need to look over what existing features we use from JSON.NET. i. There are 71 references to JSON.NET namespace in MSAL and tests projects. c. Ideally individual usages should be consolidated in JSON helper class and used from there. d. Add pre-processor flags to load the relevant implementations based on the target framework. e. Existing tests should be enough, but coverage should still be verified. f. ETA is at least 3+ days. Research of current used functionality is the most important part. Code changes should be easy once migration steps are defined.
-
Replace Newtonsoft.Json with System.Text.Json source code from https://github.com/dotnet/runtime/tree/main/src/libraries/System.Text.Json. a. The current System.Text.Json minimal requirements might not be a functional limitation and the code might simply work in 4.5.2 since it works in net 4.6.1 b. Same as above, except JSON.NET code will be removed.
-
Use System.Text.Json generator code a. We could pull the reflection less code generator into the repo and run it outside of minimal generation
-
Write your own code generation based on contract classes: a. Move json manipulation into data contract classes or multi b. Create a build task that would generate json string serializers using JsonDocument writers etc.
One of the challenges in estimating time are the unknowns that need to be researched - what exactly needs to be excluded in JSON.NET and making sure System.Text.Json supports everything that we use from JSON.NET.
Unity related work: [Feature Request] [L] Stop using reflection to deserialize JSON · Issue #2343 Update JSON.NET code to work with Unity. by pmaytak · Pull Request #2428 Remove the HAVE_CAS define so that Newtonsoft.Json never uses dynamic code generation as its not supported in Unity. by najadojo · Pull Request #2588
System.Text.Json framework support: .NET API Catalog (apisof.net)
- Built-in: .NET 5+, .NET Core 3+
- Via Nuget package: .NET Core 2+, .NET Fx 4.6.1+, .NET Standard 2+
azchohfi
Sergio0694
michael-hawker
pmaytak
tipa
mjcheetham
charlesroddie
.NET6 added IL Trimming (https://devblogs.microsoft.com/dotnet/announcing-net-6/#il-trimming) Unfortunately, MSAL.Net doesn't support it.
Which version of MSAL.NET are you using? Microsoft.Identity.Client 4.44.0
Platform .NET 6
What authentication flow has the issue?
Is this a new or existing app? This is a new app or experiment.
Repro
A very simple .NET 6 app:
With:
And built with:
dotnet publish --self-contained -r win-x64 /p:Configuration=Release /p:TrimmerSingleWarn=falseWill produce dozens of warnings/errors, such as:
Expected behavior No build errors or warnings when building a self-contained exe.
Actual behavior Multiple errors, and very hard to workaround/fix manually.
Possible solution Most errors seems related to JSON deserialization, and .NET6 now supports source generation for this, so it should be possible to support this.