Fixes Snackbar and Toast on iOS release builds

[mjo151]

Description of Change

Fixes Snackbar and Toast on iOS release builds

Linked Issues

• Fixes [BUG] Toast.Show crashing on iOS Release builds #1752

PR Checklist

• Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
• Has tests (if omitted, state reason in description)
• Has samples (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Changes adhere to coding standard
• Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

Removed the use of FrozenSet in the iOS AlertView. FrozenSet was causing the crash on iOS release builds, and it also adds unnecessary overhead when initializing the AlertView.

[mjo151]

@dotnet-policy-service agree

…

On Mar 20, 2024, at 12:28 PM, dotnet-policy-service[bot] ***@***.***> wrote: @mjo151 <https://github.com/mjo151> please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information. @dotnet-policy-service agree [company="{your company}"] Options: (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer. @dotnet-policy-service agree (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer. @dotnet-policy-service agree company="Microsoft" Contributor License Agreement <http://www.opensource.org/> — Reply to this email directly, view it on GitHub <#1767 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA5A5TIJY5OROZDNWTTEDULYZG2LVAVCNFSM6AAAAABE72DVKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBZHE4TENRQHE>. You are receiving this because you were mentioned.

[brminnick]

Thanks @mjo151.

However, this doesn't fix the the two root causes:

1. <MtouchInterpreter>-all</MtouchInterpreter> fails to include the System.Collections.Immutable assembly: [BUG] Toast.Show crashing on iOS Release builds #1752 (comment)
2. The Mono Interpreter is currently required for Release builds: Consider enabling the interpreter by default for release builds dotnet/maui#13019 (comment)

I think the best path forward for us on the Toolkit team is the following:

1. Update our documentation to include the following code as a requirement to use Snackbar + Toast as discussed with @bijington: [BUG] Toast.Show crashing on iOS Release builds #1752 (comment)

   <PropertyGroup Condition="$(Configuration.Contains('Release')) And $(TargetFramework.Contains('ios'))">
       <MtouchInterpreter>-all,System.Collections.Immutable</MtouchInterpreter>
   </PropertyGroup>

2. Monitor the .NET MAUI / .NET iOS teams progress on the Mono Interpreter: Consider enabling the interpreter by default for release builds dotnet/maui#13019 (comment)

[mjo151]

Thanks @mjo151.

However, this doesn't fix the the two root causes:

1. <MtouchInterpreter>-all</MtouchInterpreter> fails to include the System.Collections.Immutable namespace: [BUG] Toast.Show crashing on iOS Release builds #1752 (comment)
2. The Mono Interpreter is currently required for Release builds: Consider enabling the interpreter by default for release builds dotnet/maui#13019 (comment)

I think the best path forward for us on the Toolkit team is the following:

1. Update our documentation to include the following code as a requirement to use Snackbar + Toast as discussed with @bijington: [BUG] Toast.Show crashing on iOS Release builds #1752 (comment)

   <PropertyGroup Condition="$(Configuration.Contains('Release')) And $(TargetFramework.Contains('ios'))">
       <MtouchInterpreter>-all,System.Collections.Immutable</MtouchInterpreter>
   </PropertyGroup>

2. Monitor the .NET MAUI / .NET iOS teams progress on the Mono Interpreter: Consider enabling the interpreter by default for release builds dotnet/maui#13019 (comment)

@brminnick

This PR does fix the root cause of the crash on iOS, which is the use of FrozenSet in the AlertView. The AlertView is used by Snackback and Toast. As you can see from the changes in my PR, the use of FrozenSet is completely unnecessary in this case. I don't think it makes sense to force users to enable the interpreter, which has other implications, when this problem can simply be fixed by removing the use of FrozenSet.

[brminnick]

I understand that. This is a workaround to a bug in the .NET Runtime and in the Mono Interpreter.

It also includes a breaking change to our public API surface.

[mjo151]

I understand that. This is a workaround to a bug in the .NET Runtime and in the Mono Interpreter.

I agree that not being able to use System.Collections.Immutable appears to be a bug in the .NET Runtime and/or the mono AOT compiler. This PR does "workaround" that bug by not using System.Collections.Immutable. However, use of System.Collections.Immutable here was unnecessary anyway and also had negative performance implications on the code.

For various reasons, I am not able to enable the Mono Interpreter for my project. It's very unfortunate if that will be the stance for fixing issues moving forward, especially when there are other options. I really hope you review and consider this PR for integration into the community toolkit. I've been very happy with the toolkit so far, but if using Snackbar and Toast will require enabling the interpreter (e.g. this PR doesn't get approved), I will have to migrate away from using the toolkit.

[mjo151]

OK, I understand the issue with breaking the public API. I just pushed another commit that reverts the breaking change to the public API. It still resolves the crash when displaying Snackbar and Toast.

[mattleibow]

Will this cause issues if the collection changes during this loop? Before it was a copy, now it is the actual collection and IEnumerable throws if you change it while it is iterating.

[brminnick]

It's a possibility, but the likelihood is slim.

As Vlad pointed out in our standup today, Snackbar + Toast typically only contain 2-3 children, so enumerating through the list should be very quick. The odds of the List being modified during enumeration are very low.

[mjo151]

Prior to this change, the foreach loop accessed the Children property, which calls children.ToFrozenSet(). ToFrozenSet() has to iterate over the collection.

In either case, AlertView is a UI control and should only ever be modified on the UI thread. If another thread is adding children to the view, that's a bug in the consumer of AlertView.

[RobertStefancic]

Is there a chance we get a hotfix release for this soon? Or maybe a preview release?

[plppp2001]

Is there a chance we get a hotfix release for this soon? Or maybe a preview release?

I would like this also, and when could it be released? I can't use pre releases in production env.