UI DependencyProperty/AttachedProperty Property Source Generator

[michael-hawker]

Preface

This topic comes up a lot and wanted to centralize the current thoughts and proposals around it for what we want to see in the Toolkit in the future.

First off, we're waiting on support in C# for partial properties; so, until that is implemented in C#, we don't have plans to do anything in this space. Please up-vote/react to that issue in the meantime.

Also, in the meantime, feel free to look at alternatives.

Props to @Sergio0694 for working with me discussing these and iterating on our ideas and proposals for the below.

Problem

Writing properties for UI code is cumbersome, clunky, and error-prone with many pieces that need to be aligned by type, class names, and/or 'mystically' named in order to work properly (e.g. including Property at the end).

It'd be great instead, if we could leverage the power of source generators to perform this heavy lifting for us. And also gain the benefit of analyzers to ensure we set them up with proper names, semantics, types, etc... (more below)

DependencyProperty

For instance, with a Dependency Property, go from this:

    public int MyProperty
    {
        get { return (int)GetValue(MyPropertyProperty); }
        set { SetValue(MyPropertyProperty, value); }
    }

    // Using a DependencyProperty as the backing store for MyProperty.  This enables animation, styling, binding, etc...
    public static readonly DependencyProperty MyPropertyProperty =
        DependencyProperty.Register("MyProperty", typeof(int), typeof(OwnerClass), new PropertyMetadata(0, MyProperty_PropertyChangedCallback));

    private static void MyProperty_PropertyChangedCallback(DependencyObject d, DependencyPropertyChangedEventArgs e)
    {
        // TODO
    }

API Proposal

    [DependencyProperty]
    public partial int MyProperty { get; set; }

    // Optional, won't be registered automatically if not used for perf gain
    static partial void OnMyPropertyChanged(OwnerClass @this, DependencyPropertyChangedEventArgs e)
    {
    }

Super simple! 🎉

Open Questions

• What if you want multiple properties to share the same callback?
• The above doesn't account for specifying the default value of the property passed into the PropertyMetadata constructor
  • By default, we should use the base/default value for the type of the property, but should allow to override (though see note on analyzer below for best practices)
  • We should also provide a way to invoke the factory method pattern to create default values.
• Should the function just provide oldValue and newValue instead of the event args? (though, for the above question of sharing a method it would need the DP value too)
  • This was added as an option for the partial methods in the MVVM Toolkit as well, so maybe we have both as options here too?

AttachedProperty

The boilerplate for AttachedProperty is even worse:

    public static int GetMyProperty(DependencyObject obj)
    {
        return (int)obj.GetValue(MyPropertyProperty);
    }

    public static void SetMyProperty(DependencyObject obj, int value)
    {
        obj.SetValue(MyPropertyProperty, value);
    }

    // Using a DependencyProperty as the backing store for MyProperty.  This enables animation, styling, binding, etc...
    public static readonly DependencyProperty MyPropertyProperty =
        DependencyProperty.RegisterAttached("MyProperty", typeof(int), typeof(OwnerClass), new PropertyMetadata(0, MyProperty_PropertyChangedCallback));

    private static void MyProperty_PropertyChangedCallback(DependencyObject d, DependencyPropertyChangedEventArgs e)
    {
        // TODO
    }

However, since Attached Properties don't actually have a public property (it's defined as a public field)... a similar approach won't work here.

Implementation to Avoid

Ideally, we want to still avoid attaching an attribute to the class and using a string like so:

[AttachedProperty<DependencyObject, int>("MyProperty")]
public class OwnerClass : DependencyObject
{

This has many drawbacks:

1. It uses a string and therefore makes it harder to refactor or use to track references/usage easily from the code
2. You don't have an entity to add a xmldoc too
3. Generic attributes are fun, but aren't fully supported yet and require runtime support so won't be supported 'everywhere'.

API Proposal

Instead, let's hook into some part of the code we already have to write instead:

    [AttachedProperty]
    public static partial int GetMyProperty(DependencyObject obj);

    // Again optional, won't be registered automatically if not used for perf gain, and same setup as our DP implementation!
    private static partial void OnMyPropertyChanged(OwnerClass @this, DependencyPropertyChangedEventArgs e)
    {
    }

This provides both the attachable type and the type of the 'property' in the signature, and provides a natural place to place the xmldoc. The setter and field definition would be generated along with the implementation for the partial getter method. And the callback would be optionally registered, if defined.

Open Questions

• Basically, the same ones as the dependency property...
• Should we allow similarly defining the SetMyProperty method instead in case that documentation makes more sense for a scenario? As long as both aren't defined...
  • It'd be great for the XML doc defined by the developer on the one defined to be translated into the alternate of the one generated. i.e. if you write docs for GetMyProperty, we generate both the body of that method and the signature/body of SetMyProperty, and should include the 'translation' of that getter doc to the setter doc (or maybe you can define both as partial still with docs on your own if you want to, and the generator will just fill in the bodies appropriately)...
• Technically, we don't need partial properties for this Attached Property syntax to work, but they're not as common as Dependency Properties, so it seems better to wait and work on them together at the same time.

Analyzers

With source generators, generally extra Roslyn analyzers are written to ensure proper usage and aid developers in implementing proper patterns.

To that end, we should provide some of the following checks around these new generators:

• Both: Property naming, e.g. ending in Property (AP: Starting with 'Get')
• Being defined on a DependencyObject inheritor (this also covers DependencyObjectCollection as it inherits from DO)
• Check proper type parameters on partial callbacks that look like they have the On_PropertyChanged name and vice-versa (is there a partial method with the right types, but the name doesn't match a defined property we see)
• AP: Attaching to a type that inherits from DependencyObject
• DP: Suggesting using a DependencyProperty on UI classes (inheriting from Control) instead of INotifyPropertyChanged... (see this 'mistake' a lot) - imagine we can detect a regular property that has a call to PropertyChanged? At minimum if we detect the INotifyPropertyChanged interface on a control class, we can call that out...
• DP: Following best practices/conventions for the default value (see https://learn.microsoft.com/windows/uwp/xaml-platform/custom-dependency-properties#default-value), e.g. warning if a collection is provided as a default value instead of initializing in the class constructor, per recommendations, or creating a singleton.

These would all help ensure proper usages of the properties in the proper places and aid developers in using different features (like the callback) with less room for error.

References

• https://learn.microsoft.com/windows/uwp/xaml-platform/custom-dependency-properties
• https://learn.microsoft.com/windows/uwp/xaml-platform/custom-attached-properties

[kmgallahan]

I would recommend using event arguments over multiple parameters.

see this 'mistake' a lot

Likely because it is a PITA to create DPs, as mentioned. In some cases though there may be performance concerns over using DPs over INPC, although all I have as a reference here is 3rd party anecdotes.

I currently use the linked generator and would switch to this one once provided.

[michael-hawker]

Thanks for the feedback @kmgallahan! Yeah, creating DPs is a pain compared to how easy it is to get MVVM properties now with the MVVM Toolkit. 🙂 I have heard the anecdote that DP have a bit more overhead, not sure if it's just a memory thing vs. extra CPU cycles (or both)?

Though they're required for doing certain things with VSM and Binding and such, so if you don't use them that's where I see folks hit other issues later as they try and expand what their controls do. So, it is use-case specific for sure.

Glad to hear the proposal sounds good though. We're excited about it too. Just need partial properties in C#! 🤞

[Poker-sang]

Looks fascinating. But I wonder if there is a way to make it easier to create PropertyChangedCallback method?

For example, AttachedPropertyAttribute makes the class to automatically implement an interface automatically so that PropertyChanged can be written as an override method (and then implement methods using auto-completion). Or just add a code fix and complete it with this?

[michael-hawker]

Hey, didn't see this before and was relooking at this issue. We're suggesting using partial methods here, just like the MVVM Toolkit did for the OnPropertyChanging/Changed methods they have within a class.

So, it's a similar setup, and should be discoverable in VS intellisense as well, I believe. The benefit of the partial method usage is the compiler will optimize them out if unused by the developer vs. requiring methods to be defined or part of other contracts and such with an interface or having some base empty method defined which could still be called.

[Poker-sang]

Is there any news about partial properties? Or just change the way it's implemented

[Sergio0694]

They're in the working set for Roslyn and being worked on. Cautiously optimistic they'll be in C# 13.

[Arlodotexe]

Partial properties are out with C# 13🚀

[michael-hawker]

@Sergio0694 FYI, updated the original post to link to the reference docs, add a few more open questions we had discussed/thought about, and also point out some more analyzer opportunities towards encoding guidance from the docs as best-practices for common mistakes that can be made setting up DP.

[0x5bfa]

As we plan to create a bunch of custom controls in the future, we introduced our own source gen for DP (hopefully AP too) when we introduced our own first custom control.
While it doesn't utilize partial properties this would help you guys out see what the implementation would look like

Usage:
https://github.com/files-community/Files/blob/39df64d692a83f01eb69dab96dc261a3d0bc9976/src/Files.App.Controls/ThemedIcon/ThemedIcon.Properties.cs#L9-L19

Generator:
https://github.com/files-community/Files/blob/main/src/Files.Core.SourceGenerator/Generators/DependencyPropertyGenerator.cs

Attribute:
https://github.com/files-community/Files/blob/main/src/Files.Shared/Attributes/DependencyPropertyAttribute.cs

Generated code sample:

public static readonly global::System.Windows.DependencyProperty IsToggledProperty =
    global::System.Windows.DependencyProperty.Register(
        nameof(IsToggled),
        typeof(bool),
        typeof(global::Files.App.Controls.ThemedIcon),
        new global::Microsoft.UI.Xaml.PropertyMetadata(
            (bool)true,
            (d, e) => ((global::Files.App.Controls.ThemedIcon)d).OnIsToggledPropertyChanged(
                (bool)e.OldValue,
                (bool)e.NewValue));

public bool IsToggled
{
    get => (bool)GetValue(IsToggledProperty);
    set => SetValue(IsToggledProperty, value);
}

Let me know what you guys think 🤔

[michael-hawker]

Seems similar to the existing HavenDV version we linked/compared to at the start as an existing alternative.

@Sergio0694 just finished updating the MVVM Toolkit to use partial properties (they're really amazing). I think this is upcoming on his list to look at - I'd imagine we'd ship this here in Labs as it's WinUI/Framework dependent? The two of us can sync, see if I can lend a hand too. I've ramped up a bit on analyzers this summer. Was thinking could be nice to have a fixer which will add the OnPropertyChanged partial method signature/stub for you from the DP/AP if you don't have one.

@Arlodotexe would be nice if we could multi-target for WPF too somehow, as it should be 99% the same...? Not sure the implications of that though in our project/package systems...

Though, I think with partial properties in C# now, we have everything to handle our original proposal as outlined above.

[Sergio0694]

Yes, I will use a different design than this in ours. Also because we absolutely need the properties to show up in metadata for other analyzers, or they would break a number of trim/AOT scenarios with CsWinRT. Eg. the code above would not work correctly.

[0x5bfa]

Seems similar to the existing HavenDV version we linked/compared to at the start as an existing alternative.

Oh didn't know, it took me days 😄
Either way, good to know you guys have ideas.