search

TobiasBuchholz / Plugin.Firebase

Wrapper around the native Android and iOS Firebase Xamarin SDKs
MIT License
211 stars 49 forks source link

Dotnet trimmer testing #144

Open tranb3r opened 1 year ago

tranb3r commented 1 year ago

Hi,

When migrating playground/tests to dotnet maui, the <AndroidLinkMode>Full</AndroidLinkMode> property has been removed from the release build. I don't know if it's been done on purpose, but I think you could add now the corresponding dotnet trimmer property: <TrimMode>full</TrimMode>.

Also, are you sure the Preserve attributes in the plugin code are still needed? Maybe it's worth testing it again with dotnet trimmer. What do you think?

Thanks!

tranb3r commented 1 year ago

When trimming my application, android's MyFirebaseMessagingService is removed. I can workaround that issue by disabling trimming for Plugin.Firebase.CloudMessaging. But maybe this could be fixed in the plugin's code, by ensuring the service is always preserved?

TobiasBuchholz commented 1 year ago

Okay yes, I will add the Preserve attribute to the MyFirebaseMessagingService and release it with the next Plugin.Firebase.CloudMessaging version 👍

tranb3r commented 1 year ago

Ok but I'm not 100% sure that the Preserve attribute is still the way to preserve code in dotnet maui. Can you confirm that?

TobiasBuchholz commented 1 year ago

No, I can't confirm that, I was just assuming it would work. What would be the alternative?

tranb3r commented 1 year ago

I've asked for help on discord, here is what I've understood.

The Preserve attribute is now obsolete, and the linker is ignoring it. You should instead use the DynamicDependency as explained here. The best place to add this attribute would be the entry point of the library.

So I think the proper way to fix the missing MyFirebaseMessagingService would be to add an Initialize method to CrossFirebaseCloudMessaging and decorate it with the DynamicDependency attribute.

Quaybe commented 7 months ago

Disabling trimming completely until this is resolved. The workarounds no longer work for me.

jonathanpeppers commented 2 months ago

Until this one ships:

I found putting this in your app seems to workaround the issue:

[DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(MyFirebaseMessagingService))]

Can anyone confirm if it solve the problem for them?

Is this only happening when using TrimMode=full, or does it also occur with default settings?

dartasen commented 2 months ago

@jonathanpeppers Seems like it's working just fine with DynamicDependency attribute ! Even if Preserve attribute is flagged as Obsolete, still being usable is a bit confusing since it seems to have stopped working properly

jonathanpeppers commented 2 months ago

No, I don't think we can remove [ObsoleteAttribute] as that would be a breaking change. Random Xamarin.Android class libraries (such as old NuGets) could throw TypeLoadException (or possibly even more inscrutable runtime errors).

It does seem like we could add [Obsolete(error: true)] in .NET 9, so you'd get a build error in new source code if you used [PreserveAttribute]. I'll think on it.