Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Snackbar InvalidOperationException when using dotnet trimmer #1460

Closed
2 tasks done
tranb3r opened this issue Oct 16, 2023 · 19 comments · Fixed by #1367
Closed
2 tasks done

[BUG] Snackbar InvalidOperationException when using dotnet trimmer #1460

tranb3r opened this issue Oct 16, 2023 · 19 comments · Fixed by #1367
Labels
bug Something isn't working unverified

Comments

@tranb3r
Copy link

tranb3r commented Oct 16, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Did you read the "Reporting a bug" section on Contributing file?

Current Behavior

When attempting to show a Snackbar on a Maui app for Android with dotnet trimmer full mode enabled, the following exception occurs:

System.InvalidOperationException: Unable to find Snackbar text view
  at CommunityToolkit.Maui.Alerts.Snackbar.SetMessageForView(View&, IFontManager)
  at CommunityToolkit.Maui.Alerts.Snackbar.ShowPlatform(CancellationToken )
  at MauiAppSnackbar.MainPage.OnCounterClicked(Object sender, EventArgs e)

Expected Behavior

Instead of throwing an exception, the Snackbar should be displayed.

Workaround: do not trim CommunityToolkit.Maui

<ItemGroup Condition="'$(Configuration)|$([MSBuild]::GetTargetPlatformIdentifier($(TargetFramework)))'=='Release|android'">
	<TrimmerRootAssembly Include="CommunityToolkit.Maui" RootMode="All" />
</ItemGroup>

Steps To Reproduce

  1. Open the repro project
  2. Launch in release mode (dotnet trimmer full mode)
  3. Click on button: a dialog will appear, catching the exception that is raised by CommunityToolkit.Maui

Link to public reproduction project repository

https://github.com/tranb3r/Issues/tree/main/MauiAppSnackbar

Environment

- .NET MAUI CommunityToolkit: 6.0.0
- OS: Android
- .NET MAUI: 8.0 rc2

Anything else?

No response

@tranb3r tranb3r added bug Something isn't working unverified labels Oct 16, 2023
@pictos
Copy link
Member

pictos commented Oct 16, 2023

@tranb3r The MCT isn't linker friendly, so when you turn the linker on, you should take care on what Linker shouldn't remove. You can create a xml file with the methods and assemblies that you want to keep, as you can see here

@tranb3r
Copy link
Author

tranb3r commented Oct 16, 2023

@pictos
MCT is now pretty much the only third-party lib I'm using that is still not linker friendly. Is there a plan to improve this?
For the specific issue of snack-bar, right now as a workaround I'm rooting the whole MCT assembly. This is fine, but maybe I could set a more specific trimmer directive?

@tranb3r
Copy link
Author

tranb3r commented Oct 16, 2023

Also, if the MCT is not trimmer ready, maybe it should simply have its IsTrimmable attribute set. This would solve this issue.

@brminnick
Copy link
Collaborator

if the MCT is not trimmer ready, maybe it should simply have its IsTrimmable attribute set

@VladislavAntonyuk - could you add <IsTrimmable>false</IsTrimmable> to #1367?

<PropertyGroup>
    <IsTrimmable>false</IsTrimmable>
</PropertyGroup>

Here's the .NET 8 docs for it:
https://learn.microsoft.com/dotnet/core/deploying/trimming/prepare-libraries-for-trimming?pivots=dotnet-8-0#enable-project-specific-trimming

@pictos
Copy link
Member

pictos commented Oct 16, 2023

This is fine, but maybe I could set a more specific trimmer directive?

Yes, you can. Just like you should do when you have the linker enabled on Xamarin.Forms app.

Also, if the MCT is not trimmer ready, maybe it should simply have its IsTrimmable attribute set. This would solve this issue.

Not sure what that would do, if you say it will solve the issue, I believe this will skip our lib during the Linker process. And is there a way for consumers to change that configuration? I mean, let's say that I've my app and I want it to be as small as possible and trim MCT and handle the exceptions by myself. Could I do that setting it to false? Today, having that not set, consumers can just create the XML file and show that they don't want to trim the lib. That said, from a lib perspective, being more permissive here is better

@brminnick
Copy link
Collaborator

brminnick commented Oct 16, 2023

Thanks Pedro! I don't disagree.

One data point: the size of the NuGet Package for CommunityToolkit.Maui is 1.15MB and CommunityToolkit.Maui.Core is 697.48 KB. To me this size is negligible without Trimming.

That being said, if anyone wants to open a PR to enable Trimming, I'd be more than happy to review + merge it!

@tranb3r
Copy link
Author

tranb3r commented Oct 17, 2023

@pictos
Let me clarify my question.
Right now, I workaround this issue by preserving the whole MCT assembly.
However it's quite big. So I'd like to set a more specific directive, targeting this snackbar exception.
But so far I haven't been able to figure out which class/method/... should be added to the trimmer descriptors file.
Any idea?

@tranb3r
Copy link
Author

tranb3r commented Oct 17, 2023

By the way, the descriptors doc is here: https://github.com/dotnet/linker/blob/main/docs/data-formats.md

@pictos
Copy link
Member

pictos commented Oct 17, 2023

@pictos Let me clarify my question. Right now, I workaround this issue by preserving the whole MCT assembly. However it's quite big. So I'd like to set a more specific directive, targeting this snackbar exception. But so far I haven't been able to figure out which class/method/... should be added to the trimmer descriptors file. Any idea?

@tranb3r this is more a cat/ mouse game... Like the error will give you a hint about what is missing, in this case, is the Snackbar text view (from your report), then you need to dig into the code and see where this is located and preserve it on nuget... Another thing is to build in release and inspect/decompile the CommunityToolkit.Maui.dll and see what's missing and including. Since XF I didn't know any other (easy) way to do it. Grab a drink and hope you find this challenge fun

@tranb3r
Copy link
Author

tranb3r commented Oct 18, 2023

@pictos
I know the game, I've been doing it for many years.
However, for this specific exception, I've not managed to find out what is missing. So I was hoping someone could help me.
I understand you do not have the time for that. Maybe we can keep this issue open until someone can find the solution?

@tranb3r
Copy link
Author

tranb3r commented Oct 23, 2023

@pictos
I've investigated this issue a bit more.
There is no issue when building for net7. This is a regression for net8.
I think the issue is related to the new resource designer assembly. This would explain why no matter how I try to "preserve" MCT code, the issue is still here, unless I root the whole assembly.
What do you think? Maybe this issue could be transfered to the xamarin-android team?

@pictos
Copy link
Member

pictos commented Oct 23, 2023

@tranb3r thanks for looking into it, we don't support .net8 bits yet, so not sure if could be that... Maybe, when we set our TFM to net8 the issue goes away, but I don't know.

Hey @jonathanpeppers, do you know something that may changed on Linker on the latest android bits?

@jonathanpeppers
Copy link

It could be related to this, which will be fixed in .NET 8 GA: dotnet/android@03018e0

There might be an intermediate release before GA, where we have a chance to ship this change.

@brminnick brminnick mentioned this issue Nov 4, 2023
6 tasks
@tranb3r
Copy link
Author

tranb3r commented Nov 5, 2023

It could be related to this, which will be fixed in .NET 8 GA: xamarin/xamarin-android@03018e0
There might be an intermediate release before GA, where we have a chance to ship this change.

@jonathanpeppers
I've built my app with the new version of xamarin-android (34.0.0-rc.2.479) and the issue is still here.

@brminnick
Why did you close this issue? How is this bug related to the .NET8 migration of the toolkit?

@brminnick
Copy link
Collaborator

@brminnick
Why did you close this issue? How is this bug related to the .NET8 migration of the toolkit?

In #1367, we added <IsTrimmable>false</IsTrimmable> making the .NET MAUI Community Toolkit safe to use with the Trimmer.

We discussed this in this Issue above: #1460 (comment)

Next time, please review the PR code and the comment history in the Issue before tagging me. We are all volunteers working on the CommunityToolkit; we don't get paid by Microsoft to do this work, and our time is limited.

<IsTrimmable>false</IsTrimmable>

@tranb3r
Copy link
Author

tranb3r commented Nov 9, 2023

Sorry for tagging you but as the issue wa closed it was probably the only way to get your attention.
As this issue and the fix are not at all related to net8, I honestly thought it deserved some kind of explanation. Maybe next time it would be better to do a separate PR in a similar situation.
Also, I regret that the issue was closed without a proper investigation. This is probably hiding a bug in xamarin-android, as JP was suspecting. So I was sincerly hoping that someone could come up with a better fix. Even if IsTrimmable=false is fine for now.
Finally, I'm a long time xamarin user, and I have to say that MCT is great. I'm really thankful for your time and your work.

@jonathanpeppers
Copy link

Note that setting IsTrimmable=false is not a long-term solution. It will not work on NativeAOT for iOS, for example, as NativeAOT has to trim everything. It will completely ignore this setting in this case.

Details at: dotnet/maui#18658

@brminnick
Copy link
Collaborator

Noted- thanks @jonathanpeppers!

I’ll open an Issue and we’ll certainly be sure to add Trimmer support before iOS moves to NativeAOT in .NET 9 💯

@tranb3r
Copy link
Author

tranb3r commented Nov 15, 2023

This issue is fixed in version 7.0.0 of CommunityToolkit.Maui.
Probably thanks to the bug fix in xamarin-android mentionned by Jonathan.

Please note that the added IsTrimmable=false does not seem to have any effect. The assembly-level attribute is not set.
This is totally fine in the context of this issue: the bug is fixed and we can still trim the assembly.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working unverified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants