Telegram.Bot icon indicating copy to clipboard operation
Telegram.Bot copied to clipboard

Support AOT/Trimming/Blazor WebAssembly using source generation

Open cyrmax opened this issue 2 years ago • 11 comments

I would like to request/propose a feature

I suggest replacing NewtonSoft.Json with System.Text.Json with proper source generation used. The reason is that with newtonsoft we cannot use Aot and/or trimming which then breaks the entire Json package. With System.Text.Json we can see the similar issue with trimming and Aot. But there we can use source generation aproach and avoid any errors when trimming the executable or building native code. Here is the documentation for this source generation feature in detail and System.Text.Json docs can be googled easily. https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/source-generation?pivots=dotnet-8-0

If anybody has any arguments against my feature request I would be glad to discuss them. Also I will try to implement the thing I want by myself and if I achieve what I want without any errors I of course will make a pull request to address this issue.

Thanks in advance.

cyrmax avatar Dec 11 '23 21:12 cyrmax

@cyrmax wow, what a great idea! How would you approach this transformation with the existing codebase?

karb0f0s avatar Dec 12 '23 09:12 karb0f0s

Will this work for netstandard2.0 targets? Old runtimes that support it is still widely used and we can't stop supporting it yet.

tuscen avatar Dec 12 '23 16:12 tuscen

@tuscen I found the following information regarding your question. TLDR: yes. But here is a quote and a link.

The System.Text.Json library is included in the runtime for .NET Core 3.1 and later versions. For other target frameworks, install > the System.Text.Json NuGet package. The package supports: • .NET Standard 2.0 and later versions • .NET Framework 4.7.2 and later versions • .NET Core 2.0, 2.1, and 2.2

And here is the link: https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/migrate-from-newtonsoft?pivots=dotnet-8-0

cyrmax avatar Dec 12 '23 18:12 cyrmax

@cyrmax Thanks, we'll probably try to migrate to source generated converters then

tuscen avatar Dec 12 '23 18:12 tuscen

@karb0f0s If you mean that it is impossible or too difficult then please explain why. I didn't looked at the code yet but I think at least it is possible and reasonable.

cyrmax avatar Dec 12 '23 18:12 cyrmax

@cyrmax By the way, you really don't want this library to be trimmed since it's an API client. What will it do in case your bot receives an object that should be deserialized to a type that was trimmed during compilation?

tuscen avatar Dec 12 '23 18:12 tuscen

@cyrmax Do you have experience with STJ source generators? I’ve tried to use it in this library, but for some reason it doesn’t generate converters. But in a separate test project I don’t have any problems.

tuscen avatar Dec 14 '23 22:12 tuscen

@tuscen I will try to investigate this on weekend. In my projects this feature works just fine too. I will see.

cyrmax avatar Dec 15 '23 11:12 cyrmax

I've managed to generate json converters, but I struggle to understand how to make it work in .NET 6 in such a way that one could augment existing JsonSerializerOptions instead of completely rewriting existing ones. This is a deal breaker since we can't impose our own instance of JsonSerializerOptions on clients using webhooks since they might have their own conventions in their ASP.NET Core apps. It seems .NET 8 have a solution for this using JsonTypeInfoResolver.Combine which .NET 6 lacks. If you have ideas on how to workaround please tell me. Otherwise I don't think we will be able to use STJ until .NET 6 is EOL so we could target the library to .NET 8 instead of .NET 6.

tuscen avatar Dec 18 '23 18:12 tuscen

I've managed to generate json converters, but I struggle to understand how to make it work in .NET 6 in such a way that one could augment existing JsonSerializerOptions instead of completely rewriting existing ones. This is a deal breaker since we can't impose our own instance of JsonSerializerOptions on clients using webhooks since they might have their own conventions in their ASP.NET Core apps. It seems .NET 8 have a solution for this using JsonTypeInfoResolver.Combine which .NET 6 lacks. If you have ideas on how to workaround please tell me. Otherwise I don't think we will be able to use STJ until .NET 6 is EOL so we could target the library to .NET 8 instead of .NET 6.

https://www.planetgeek.ch/2022/10/15/using-system-text-json-alongside-newtonsoft-json-net/ In this article author shows how to use System.Text.Json alongside Newtonsoft. And also he shows how it's possible to change serializer options for endpoints marked with special attribute (file SystemTextJsonBodyModelBinder.cs lines 10-11)

Maybe it would be useful for further research.

tarasverq avatar Jan 03 '24 21:01 tarasverq

We are now using System.Text.Json in recent versions of the library. I keep this issue open to follow up on AOT/Trimming/Source generation compatiblity

wiz0u avatar Jul 10 '24 12:07 wiz0u