InvalidArgumentException raised by ExpressionNode in _release_ mode

[vgromfeld]

Describe the bug

The animation expression builder is throwing an InvalidArgumentException in release mode when removing all the .Net Native directives from default.rd.xml.

Steps to Reproduce

Steps to reproduce the behavior:

1. Get the attached sample app
   ExpressionBuiilder.zip
2. Compile in release
3. Run the app

Expected behavior

The apps works (as when compiled in debug mode).

Additional context

The generated name for some properties is not valid.
Here is what we get in debug:

Here is what we have in release:

The issue comes from https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/1e0f17415eae2fd69dc207674b6019ed8656fec2/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs#L291

Which is not returning a valid string for the composition API. We should remove the : from the returned strings (the .Net Native generated type name when all the reflection code is removed).

Workaround

The current workaround is to force .Net Native to keep all the symbols for Windows.UI.Composition.CompositionPropertySet by using the following Default.rd.xml configuration:

<Directives xmlns="http://schemas.microsoft.com/netfx/2013/01/metadata">
  <Application>
    <!-- 
      The animation expression builder is not generating a valid name for the (scalar) parameters coming from the CompositionPropertySet.
      We workaround the issue by forcing the native compiler to keep the reflection information for this type.
    -->
    <Type Name="Windows.UI.Composition.CompositionPropertySet" Dynamic="Public" />
  </Application>
</Directives>

[ghost]

Hello vgromfeld, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

[michael-hawker]

FYI @Sergio0694

[michael-hawker]

@vgromfeld since you've identified the code line, did you have a chance to try changing that and seeing if it makes an improvement? Did you want to submit a PR?

[Sergio0694]

Huh, that's interesting that the missing metadata actually causes such a visible behavioral difference there 🤔
Looks like the fix is straightforward enough thankfully, good find @vgromfeld! 🥳

[michael-hawker]

It looks like we could replace the magic string with the type with what the Pipeline builder uses to create a unique string:

    // Ensure we have a unique name for each object in our expression.
    string paramName = Guid.NewGuid().ToUppercaseAsciiLetters();

It seems like this reference is pulled up from the hash when needed, so it shouldn't matter what the string is as long as it's unique and confines to the requirements for the Comp APIs.

@Sergio0694 said he's going to optimize that method and move it to a central location for both places to use.