bitwarden / passwordless-dotnet

Bitwarden Passwordless.dev .NET SDK.
https://bitwarden.com/
Apache License 2.0
33 stars 10 forks source link

Add support for assembly trimming and AOT #132

Open Tyrrrz opened 3 months ago

Tyrrrz commented 3 months ago

130 added annotations for APIs that dynamically reference code (usually through reflection). These methods are, in essence, in compatible with trimming/AOT.

This means that the only available way to set up the raw Passwordless.dev SDK is by using this overload, which requires configuring the options manually:

public static IServiceCollection AddPasswordlessSdk(
        this IServiceCollection services,
        Action<PasswordlessOptions> configureOptions)

For the Identity integration package, we currently offer no way to add Passwordless.dev in a way that works with trimming/AOT.

Some of it may be a side-effect of the nature of ASP.NET Core as the foundation, but Microsoft is already rolling out certain alternative APIs to help support trimming/AOT scenarios. For example, for the configuration binding, we can use this approach to avoid reflection.

abergs commented 3 months ago

While this issue is informative, are there any clear steps of action? While intriguing, I don't see AOT & trimming highly prioritised by users (since they run on top of asp.net)

Tyrrrz commented 3 months ago

ASP.NET Core does support AOT as of .NET 8 (at least the docs claim so): https://learn.microsoft.com/en-us/aspnet/core/fundamentals/native-aot?view=aspnetcore-8.0

The benefits are mostly: smaller disk footprint (extra useful for docker images) and faster execution.

As you can see from this table, not every aspect of ASP.NET is fully supported:

image

When it comes to actionable steps, I think we should look into providing overloads that are compatible with source-generated configuration binding. Most of the trim warnings that are triggered in our code are due to IConiguration.Bind(...) or similar calls. We should instead accept an IOptions<...> directly as a parameter.

Ideally, we should refactor our code so that the only trim warnings we have are those that bubble up from our dependencies, specifically those from ASP.NET Core and Identity that don't have trim-compatible alternatives. Currently, this is not the case.

I agree that AOT/trimming is a relatively low priority issue for web apps, but it's not unlikely that it will not remain such indefinitely.