Deprecated StringExtensions.AsFormat extension

[Sergio0694]

PR Type

What kind of change does this PR introduce?

• Deprecation

What is the current behavior?

The Microsoft.Toolkit.StringExtensions.AsFormat extension can be replaced with C# string interpolation and is not needed.

What is the new behavior?

The extension has been marked as deprecated.

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link:
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

[ghost]

Thanks Sergio0694 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[michael-hawker]

I think I had added this for x:Bind, does String Interopolation work there?

[Sergio0694]

Oh, I see. So, it should work, and it does, but it's currently bugged (see microsoft/microsoft-ui-xaml#2654).

For instance, this works fine:

<TextBlock Text="{x:Bind x:String.Format('Name: {0}', ViewModel.Name)}" />

But using more arguments results in a build-time error from the XAML compiler because it's doing an incorrect overload resolution (basically it's just picking whatever the first overload is alphabetically), so it ends up picking the one taking an IFormatProvider instead of a string, and then failing to build 😟

Let me know how you'd like to proceed, I'm fine with closing this if you think the extension should remain there for this reason. I wish this was just fixed on the XAML compiler end, as the current overload resolution is really not great.

[michael-hawker]

So, will this helper work around this bug or face the same issue?

If it helps get around the issue, then maybe let's leave it for now until we know that we can get a fix in WinUI 3?

[Sergio0694]

Well this is a bit annoying, I just checked with this extension and it seems the XAML compiler still chokes up anyway because it doesn't account for a params T[] parameter allowing multiple inputs. As in, this fails too:

<TextBlock Text="{x:Bind wct:StringExtensions.AsFormat('{0} {1}', ViewModel.Name, ViewModel.Surname)}"/>

With this error:

Invalid binding path 'wct:StringExtensions.AsFormat('{0} {1}', ViewModel.Name, ViewModel.Surname)' : Missmatched function parameter count.

It just works with a single parameter basically, just with string.Format as usual. It seems to me due to the quirks of the XAML compiler we can then basically deprecate the extension without losing any actual functionality? 🤔

[michael-hawker]

@Sergio0694 yeah, let's deprecate, but point to the WinUI issue for reference in addition.

[Sergio0694]

Not sure where to add the link to the WinUI issue, since that's XAML specific and the Microsoft.Toolkit is platform agnostic instead (as in, I thought we wanted to avoid platform specific notes in these packages). Do you want me to just leave a link here in the PR or should I add that to the XML docs of the extension anyway?

[michael-hawker]

Maybe just in the code then vs. anything publicly visible? Just for our own knowledge within the toolkit?

[Sergio0694]

Sure, done in 1fb8c50 🙂