TelegramBots / Telegram.Bot

.NET Client for Telegram Bot API
https://telegrambots.github.io/book
MIT License
3.15k stars 684 forks source link

Use source generation to support AOT/Trimming/Blazor WebAssembly #1322

Open cyrmax opened 9 months ago

cyrmax commented 9 months ago

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.

karb0f0s commented 9 months ago

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

tuscen commented 9 months ago

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

cyrmax commented 9 months ago

@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

tuscen commented 9 months ago

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

cyrmax commented 9 months ago

@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.

tuscen commented 9 months ago

@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 commented 8 months ago

@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.

cyrmax commented 8 months ago

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

tuscen commented 8 months ago

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.

tarasverq commented 8 months ago

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.

wiz0u commented 1 month ago

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