Sergio0694 commented 3 years ago

Proposal: TypeConverter-equivalent for class properties

Summary

In WPF, it's possible to define a TypeConverter type to manually implement conversion logic from a string to arbitrary types when parsing XAML code. This is not supported on UWP. UWP does have the [CreateFromString] attribute, but that only works for custom types. This is not a replacement for TypeConverter, as it misses one important use case: being able to implement parsing logic for types you don't own, when used as properties in types you do own.

Example

I'm working on a rewrite for the Microsoft.Toolkit.Uwp.UI.Animations package (here), and I have a bunch of classes that would ideally expose properties of types such as Vector3 and Vector4. The XAML compiler currently does not support parsing these types automatically on custom types. But even if it did, that would not be a solution, as developers need to be able to define parsing logic themselves (we can't just assume they'll never have to parse some type that's already built-in in the XAML parser).

Consider this simple example:

public sealed class TranslationAnimation
{
    public Vector3? From { get; set; }
    public Vector3? To { get; set; }
}

This will just fail to parse in XAML, and it will crash. I can't use [CreateFromString], because I do not own Vector3. There are currently two solutions:

1) Add a markup extension that converts string to Vector3?. This retains type safety on the class, but makes the code very verbose, as now consumers will always need to do From="{ex:Vector3Extensions Value=20,0,0}" instead of just From="20,0,0". This is too much to ask to consumers for every single usage, and that's why I didn't go for this solution in the Toolkit either. 2) Introduce a generic type in the class architecture that allows specifying parsing type and parsed type, so that for types not supported by XAML, such as this one, classes can just use string as public type and then implement a custom logic. Something like this, which is untimately what I did in that PR:

public abstract class Animation<TValue, TKeyFrame>
    where TKeyFrame : unmanaged
{
    public TValue? To { get; set; }
    public TValue? From { get; set; }

    protected abstract (TKeyFrame? To, TKeyFrame? From) GetParsedValues();
}

public sealed class TranslationAnimation : Animation<string, Vector3>
{
    protected override (Vector3?, Vector3?) GetParsedValues()
    {
        return (To?.ToVector3(), From?.ToVector3());
    }
}

Consumers can now use the nice compact syntax in XAML, and classes can handle all the parsing logic. Now, this works, but it's... Pretty terrible for a number of reasons: 1) You're forced to introduce extra complexity (more type parameters, extra abstract parsing methods, etc.) 2) The actual type of values in the animation is... Wrong. This TranslationAnimation will expose To and From as string values instead of just Vector3? values as it should be. This makes it pretty bad in cases where users might have wanted to actually set a concrete Vector3 value directly, as they'll have to round-trip to a string first.

I'm not super familiar with the various WinRT limitations that could come into play with implementing a solution here, but really any sort of solution that would allow consumers to just add an attribute over properties to indicate how to parse them would work fine. Either:

Add support for [TypeConverter]
Allow [CreateFromString] to be used as an attribute over properties (and not just type)
Some new attribute specifically for properties

Any of these 3 solutions or an equivalent one would work just great, really 😄

Rationale

Allow consumers to fill in where the built-in XAML parser doesn't support types
Allow consumers to customize the XAML parsing behavior for properties
Reduce the code complexity to work around this limitation today

Scope

Capability	Priority
Allow developers to annotate properties to supply a custom parsing method	Must

API example/proposal

Here's an example of how this could work in case [CreateFromString] support for properties was introduced:

public sealed class TranslationAnimation
{
    [CreateFromString(MethodName = "Microsoft.Toolkit.Uwp.UI.Extensions.VisualExtensions.ToVector3")]
    public Vector3? From { get; set; }

    [CreateFromString(MethodName = "Microsoft.Toolkit.Uwp.UI.Extensions.VisualExtensions.ToVector3")]
    public Vector3? To { get; set; }
}

Usage:

<TranslationAnimation From="0" To="20,0,0" />

Perfect! 🎉

StephenLPeters commented 3 years ago

@ryandemopoulos seems like a cool idea

MichaeIDietrich commented 3 years ago

Optionally one could think about separating the source type information from the MethodName property, to make such references to break less likely on code refactorings:

public sealed class TranslationAnimation
{
    [CreateFromString(SourceType = typeof(VisualExtensions), MethodName = nameof(VisualExtensions.ToVector3))]
    public Vector3? From { get; set; }

    [CreateFromString(SourceType = typeof(VisualExtensions), MethodName = nameof(VisualExtensions.ToVector3))]
    public Vector3? To { get; set; }
}

Sergio0694 commented 3 years ago

@MichaeIDietrich That certainly would be a welcome addition, though since that change would apply to the [CreateFromString] attribute as it already works today (as in, that would work when used normally over custom types too), I think that should go in a differeent proposal, to keep this one as small scoped as possible, so that it's easier to work on for the team 🙂

MikeHillberg commented 3 years ago

I like the approach of [CreateFromString] supporting the property case.

What I don't like about type converters is that there's no type information telling you what input types are supported or what type you're going to get out. With CreateFromString it's more clear (though you still don't know the specific string syntax). It's also nice that CreateFromString is clear as to how to get the same functionality from code as from markup (you can do the same with a TypeConverter but the code's more cumbersome and less obvious and not typed).

Sergio0694 commented 3 years ago

Hey @MikeHillberg, figured I'd post a small update with some more info about our specific use case.

My primary use for this feature would be to use it to finally fix the typing system in our animation types in the Windows Community Toolkit, specifically given that this would be a breaking change I could see us adopting such a feature in a new major release only targeting WinUI 3 (as this change doesn't seem likely to ever be backported to UWP anyway). I've added a summary of our current animation architecture in the first post of this issue, including its many drawbacks (multiple type parameters, "fake" strong typing, etc.). If support for [CreateFromString] was extended to properties like you mentioned, I reckon I would be able to change:

public abstract class Animation<TValue, TKeyFrame>
    where TKeyFrame : unmanaged
{
    public TValue? To { get; set; }
    public TValue? From { get; set; }

    protected abstract (TKeyFrame? To, TKeyFrame? From) GetParsedValues();
}

public sealed class TranslationAnimation : Animation<string, Vector3>
{
    protected override (Vector3?, Vector3?) GetParsedValues()
    {
        return (To?.ToVector3(), From?.ToVector3());
    }
}

Into:

public abstract class Animation<T>
    where T : unmanaged
{
    public abstract T? To { get; set; }
    public abstract T? From { get; set; }
}

public sealed class TranslationAnimation : Animation<Vector3>
{
    [CreateFromString(MethodName = "Microsoft.Toolkit.Uwp.UI.Extensions.VisualExtensions.ToVector3")]
    public override Vector3? To { get; set; }

    [CreateFromString(MethodName = "Microsoft.Toolkit.Uwp.UI.Extensions.VisualExtensions.ToVector3")]
    public override Vector3? From { get; set; }
}

Which would be such a massive improvement and what I wish I could have done from the start 😄 Not only the types themselves would be way less verbose due to not having to carry out that super clunky secondary type parameter (that's really only needed to make the code compile) and having to invoke a parsing method every time, but also it would make it much easier for developers to immediately understand what types they're working with, and also to make their life easier if they wanted to set some of these properties directly from either code behind or some other method, as they'd be able to just stick to concrete types instead of having to pass string instances around (hence also making code easier to maintain).

I understand there's lots of work going on with WinUI 3, but would love to know if implementing this would be complex or instead doable with relatively no big complications, and if you could see this making its way into a future WinUI 3 preview. Thanks! 🙂

FYI @michael-hawker

michael-hawker commented 3 years ago

Was just hitting this again too for shadows in the Toolkit. Since Vector3 itself is pretty prevalent in the framework, it'd be nice if there was just a built-in type converter to it from string so it can be used easily as a custom dependency property. I think this would help alleviate the concern raised in #2130 as well. It'd probably be an easier short-term win. Should a new issue be opened for that specific ask for Vector2/3?

StephenLPeters commented 3 years ago

@RealTommyKlein FYI

michael-hawker commented 3 years ago

In the cases where we're parsing static strings (vs. using binding); it'd be nice if we could have an option to pre-compute these at compile time vs. runtime too?

michael-hawker commented 1 year ago

Bumping, though think this is a feature request, based on https://github.com/microsoft/microsoft-ui-xaml/discussions/8638

microsoft / microsoft-ui-xaml