[MVUX][C# Markup] Unable to bind Custom Control's Collection Dependency Property to consumer IListState<>

[kucint]

Hi @dr1rrb.

I want to create Custom Control that exposes read only collection as Dependency Property.
Then I want to bind this property to the IListState defined by a consumer (e.g. Frame).

It seems that there are two problems here:

Problem 1: Binding read only collection Dependency Property to IListState causes compilation error:

CS1660: Cannot convert lambda expression to type 'string' because it is not a delegate type
This is due to not existing setter of Dependency Property.

Problem 2: Binding read/write collection Dependency Property to IListState causes runtime error.

if add the setter (which is wrong I suppose), the code compiles but causes a runtime exception:

Microsoft.UI.Xaml.Data.BindingExpression: Error: Failed to apply binding to property [DependencyPropertyDetails(MyStringCollection)] on [CustomControlWithCollection.Controls.MyControl] (ReferenceConverter cannot convert from Uno.Extensions.Reactive.Bindings.BindableListFeed...)

Have a look:

This is MyControl, that exposes two Dependency Properties: MyStringCollectionProperty and MyReadOnlyStringCollectionProperty.

public sealed class MyControl : Control
{
    private static readonly MyStyle _defaultStyle = new MyStyle();

    public MyControl()
    {
        DefaultStyleKey = typeof(MyControl);
        Style = _defaultStyle;

        SetValue(MyStringCollectionProperty, new List<string>());           // initializing local instance of dependency property
        SetValue(MyReadOnlyStringCollectionProperty, new List<string>());   // initializing local instance of dependency property
    }

    #region MyStringCollection
    public IList<string> MyStringCollection
    {
        get { return (IList<string>)GetValue(MyStringCollectionProperty); }
        set { SetValue(MyStringCollectionProperty, value); }
    }

    public static readonly DependencyProperty MyStringCollectionProperty =
        DependencyProperty.Register(
            nameof(MyStringCollection),
            typeof(IList<string>),
            typeof(MyControl),
            new PropertyMetadata(
                new List<string>(), // initializing static instance of dependency property
                (s, e) => ((MyControl)s)?.OnMyStringCollectionChanged(e)));

    private void OnMyStringCollectionChanged(DependencyPropertyChangedEventArgs e)
    {
        System.Diagnostics.Debug.WriteLine($"MyProperty Changed from [{((IList<string>)e.OldValue).Count}] to [{((IList<string>)e.NewValue).Count}]");
    }
    #endregion

    #region MyReadOnlyStringCollection
    public IList<string> MyReadOnlyStringCollection
    {
        get { return (IList<string>)GetValue(MyReadOnlyStringCollectionProperty); }
        //set { SetValue(MyReadOnlyStringCollectionProperty, value); } // read-only property
    }

    public static readonly DependencyProperty MyReadOnlyStringCollectionProperty =
        DependencyProperty.Register(
            nameof(MyReadOnlyStringCollection),
            typeof(IList<string>),
            typeof(MyControl),
            new PropertyMetadata(
                new List<string>(), // initializing static instance of dependency property
                (s, e) => ((MyControl)s)?.OnReadOnlyMyStringCollectionChanged(e)));

    private void OnReadOnlyMyStringCollectionChanged(DependencyPropertyChangedEventArgs e)
    {
        System.Diagnostics.Debug.WriteLine($"MyProperty Changed from [{((IList<string>)e.OldValue).Count}] to [{((IList<string>)e.NewValue).Count}]");
    }
    #endregion
}

internal class MyStyle : Style<MyControl>
{
    public MyStyle() => Setters(s => s
        .BorderBrush(Colors.Red)
        .BorderThickness(1)
        .Template(ctrl =>
        new StackPanel()
        .Children(
            new ItemsRepeater()
            .ItemsSource(x => x.TemplateBind(() => ctrl.MyReadOnlyStringCollection))
            .ItemTemplate<string>(s =>
                new TextBlock()
                .Text(s)),
            new ItemsRepeater()
            .ItemsSource(x => x.TemplateBind(() => ctrl.MyStringCollection))
            .ItemTemplate<string>(s =>
                new TextBlock()
                .Text(s))
            )
        ));
}

This is my consumer:

public sealed partial class MainPage : Page
{
    public MainPage() => this
        .DataContext(new BindableMainPageModel(), (page, vm) => page
            .Background(ThemeResource.Get<Brush>("ApplicationPageBackgroundThemeBrush"))
            .Content(new StackPanel()
            .VerticalAlignment(VerticalAlignment.Center)
            .HorizontalAlignment(HorizontalAlignment.Center)
            .Children(
                new TextBlock()
                .Text("Hello Uno Platform!"),
                new MyControl()
                .MyReadOnlyStringCollection(() => vm.MyStringCollection),  // this line causes compilation error
                new MyControl()
                .MyStringCollection(() => vm.MyStringCollection)                   // this line causes runtime error
            )));
}


internal partial record MainPageModel
{
    public IListState<string> MyStringCollection => ListState<string>.Value(this, () =>
    [
        "Item 1",
        "Item 2",
        "Item 3",
        "Item 4",
        "Item 5",
    ]);
}

Project settings:

<TargetFrameworks>
  net8.0-windows10.0.19041;
  net8.0-desktop;
</TargetFrameworks>

Tested under net8.0-desktop.

Note, that if project runs under net8.0-windows10.0.19041, different type of exception is thrown.

Complete Solution: CustomControlWithCollection.zip

[agneszitte]

cc @dansiegel, @dr1rrb

[dr1rrb]

Hey @kucint

Sorry for the delay, missed the notifications.

Problem 1:

As the property is read-only, we cannot set the collection instance neither from code, nor from binding. That's why the C# markup generator generates only an helper to add items to the existing collection instance.

In your repro the new MyControl().MyReadOnlyStringCollection(() => vm.MyStringCollection) is then invalid as there is no binding possible.

About MVUX, as the model is responsible to instanciate the collection used by the view (and maintain it in sync with notification recieved from the ListFeed), you cannot use a read-only property here.

Problem 2:

This is more tricky. Let look at the basic flow of binding engine for ControlProperty="{Binding ModelProperty}":

var source = DataContext;
var target = Control;

var sourceProperty = source.GetType().GetProperty("ModelProperty");
var targetProperty = target.GetType().GetProperty("ControlProperty");

var value = sourceProperty.Getter.Invoke(source); // Gets the value from the source
targetProperty.Setter.Invoke(TargetControl, value); // Sets the value to the target

The issue here is that we don't know the type of the value (it's an object in pseudo-code above). Binding engine tries its best to convert the value to match types between sourceProperty and targetProperty (e.g. int to string), but it's not able convert generic types like IList<int> to IList<string>.

Usually to workaround that limitation, controls dealing with list are either:

1. Working with non-generic list (like IEnumerable). That's the case for control that tries to render a list of items provided by the user, like ListView.
2. Creates concrete type for its needs, like a Grid with the ColumnDefinitionCollection which implements IList<ColumnDefinition>.

Considering that, the collection used by MVUX does not implement IList<string> (for your case), but only ICollectionView (which implies IEnumerable<object>, IList<object>, IObservableVector<object>).

But I agree that there is nothing that prevent us to also implement IList<T> for cases like yours. If so the binding would have work seamlessly. Lets track this here: #2333

So until this issue above is being completed, you can change the taype of your property to IEnumerable:

#region MyStringCollection
public IEnumerable MyStringCollection
{
    get { return (IEnumerable)GetValue(MyStringCollectionProperty); }
    set { SetValue(MyStringCollectionProperty, value); }
}

public static readonly DependencyProperty MyStringCollectionProperty =
    DependencyProperty.Register(
        nameof(MyStringCollection),
        typeof(IEnumerable),
        typeof(MyControl),
        new PropertyMetadata(
            new List<object>(), // initializing static instance of dependency property
            (s, e) => ((MyControl)s)?.OnMyStringCollectionChanged(e)));

private void OnMyStringCollectionChanged(DependencyPropertyChangedEventArgs e)
{
    System.Diagnostics.Debug.WriteLine($"MyProperty Changed from [{((IList<object>)e.OldValue).Count}] to [{((IList<object>)e.NewValue).Count}]");
}
#endregion

PS: Don't forget to also change the initial value in the constructor: SetValue(MyStringCollectionProperty, new List<object>());

[kucint]

@dr1rrb Thanks for reply. Your proposed fix is indeed working, I will wait for #2333, this let me to have typed list in the code, which is just safer.

I have one more question in this topic:
You recommend that the instance of the property must be initialized to:

• List<object>-> SetValue(MyStringCollectionProperty, new List<object>()); // before the fix
• List<T>-> SetValue(MyStringCollectionProperty, new List<string>()); // after the fix
  got it.
  Q3: but shouldn't it rather be new ObservableCollection<object>() / new ObservableCollection<string>() respectively?
  My thinking is that MyControl does not necessarily will be used with a MVUX/MVVM model that creates ICollectionView under the hood. if none of MVUX/MVVM is used, the default property value should be an ObservableCollection rather than just List, right?

[kucint]

@dr1rrb FYI:
The solution with IEnumerable works like a charm under "Uno.Sdk": "5.2.161", both net8.0-desktop and net8.0-windows10.0.19041;
However on dev branch "Uno.Sdk": "5.3.0-dev.1827" it is stable under net8.0-desktop but unstable under net8.0-windows10.0.19041; one of following may happen:

• 95% -> process starts but has internal exception, works as usual and terminates with exit code = 0
  
• 4% -> process starts but has internal exception, works as usual and terminates with exit code = 0xc0000374
• 1% -> process crashes on start

repro: UnoCollectionPropertyUnstableApp.zip

[dr1rrb]

• Q3: but shouldn't it rather be new ObservableCollection<object>() / new ObservableCollection<string>() respectively?
  My thinking is that MyControl does not necessarily will be used with a MVUX/MVVM model that creates ICollectionView under the hood. if none of MVUX/MVVM is used, the default property value should be an ObservableCollection rather than just List, right?

Well, it really depends who is the "owner" of the collection. The 2 cases are like fore the Q2:

1. If the owner is the "model" (could be MVUX, a VM of MVVM or even code-behind), you can even default the collection to null.
2. If the collection is owned by your control, you could indeed use (or inherit from) ObservableCollection<T>().

But in both cases I would recommend that you use the PropertyChangedCallback so when a collection is pushed to the control, you can check if it implements INotifyCollectionChanged, then subscribe to event and react to it.

e.g.:

public MyControl()
{
    DefaultStyleKey = typeof(MyControl);
    Style = _defaultStyle;

    Loaded += OnLoaded;
    Unloaded += OnUnloaded;
}

private static void OnLoaded(object sender, RoutedEventArgs e)
{
    if (sender is MyControl that && that.MyStringCollection is INotifyCollectionChanged ncc)
    {
        ncc.CollectionChanged -= that.OnMyStringCollectionItemsChanged;
        ncc.CollectionChanged += that.OnMyStringCollectionItemsChanged;
    }
}
private static void OnUnloaded(object sender, RoutedEventArgs e)
{
    if (sender is MyControl that && that.MyStringCollection is INotifyCollectionChanged ncc)
    {
        ncc.CollectionChanged -= that.OnMyStringCollectionItemsChanged;
    }
}

#region MyStringCollection
public IEnumerable MyStringCollection
{
    get { return (IEnumerable)GetValue(MyStringCollectionProperty); }
    set { SetValue(MyStringCollectionProperty, value); }
}

public static readonly DependencyProperty MyStringCollectionProperty =
    DependencyProperty.Register(
        nameof(MyStringCollection),
        typeof(IEnumerable),
        typeof(MyControl),
        new PropertyMetadata(
            new List<object>(), // initializing static instance of dependency property
            (s, e) => ((MyControl)s)?.OnMyStringCollectionChanged(e)));

private void OnMyStringCollectionChanged(DependencyPropertyChangedEventArgs e)
{
    if (e.OldValue is INotifyCollectionChanged oldNcc)
    {
        oldNcc.CollectionChanged -= OnMyStringCollectionItemsChanged;
    }

    System.Diagnostics.Debug.WriteLine($"MyProperty Changed from [{((IList<object>)e.OldValue).Count}] to [{((IList<object>)e.NewValue).Count}]");
    
    if (e.NewValue is INotifyCollectionChanged newNcc)
    {
        newNcc.CollectionChanged += OnMyStringCollectionItemsChanged;
    }
}

private void OnMyStringCollectionItemsChanged(object? sender, NotifyCollectionChangedEventArgs e)
{
    switch (e.Action)
    {
        case NotifyCollectionChangedAction.Add:
            System.Diagnostics.Debug.WriteLine($"Added [{e.NewItems!.Count}] items");
            break;

        case NotifyCollectionChangedAction.Remove:
            System.Diagnostics.Debug.WriteLine($"Removed [{e.OldItems!.Count}] items");
            break;
                
        // ../..
    }
}
#endregion

Note

In this example I made sure to subscribe and unsubscribe from CollectionChanged event on load / unload. This is to ensure that the collection and MyControl will not leak in any case. This is a good practice is to un-subscribe to all events on unload (especially since GC is kind of lazy on iOS). The down side is that on re-load you will have to detect changes that might have occurred while unloaded, fopr instance when navigating back on a Page that hosts you control (so you will get Loaded -> UnLoaded -> Loaded). Another solution would be to use WeakReference/CondtionalWeakTable to make sure that your event handler does not create strong reference to your control.

(Noticed your issue with SDK 5.3.0-dev.1827, investigating)

[kucint]

@dr1rrb oh, I see what you mean, this is clever. Thanks!

Concerning (Noticed your issue with SDK 5.3.0-dev.1827, investigating)
I guess it has something to do with this line: private static readonly MyStyle _defaultStyle = new MyStyle();
See #2349, it produces similar symptoms.

[dr1rrb]

Concerning (Noticed your issue with SDK 5.3.0-dev.1827, investigating) I guess it has something to do with this line: private static readonly MyStyle _defaultStyle = new MyStyle(); See #2349, it produces similar symptoms.

Replied on #2349 as looks like to be the root cause.