Make binding internals a private API

[grokys]

Our binding internals need a refactor, for a number of reasons (which I won't get into here), but we probably won't have chance to do this before 11.0, and so it will need to be done during the 11.0 timeframe.

The problem with that is that much of our binding internals are exposed as a public API meaning that we won't be able to refactor it without breaking our API promises. For that reason I think we'll need to make much of our binding internals a private API for 11.0, in particular the following classes:

• InstancedBinding
• BindingExpression
• ExpressionObserver
• ExpressionNode and derived classes
• The concrete implementations of IPropertyAccessorPlugin
• The concrete implementations of IDataValidationPlugin
• The concrete implementations of IStreamPlugin

[jp2masa]

I use InstancedBinding on some code, wouldn't it be possible to mark it as obsolete for example?

[grokys]

Thanks for the feedback @jp2masa - that's good to know. Would an alternative API, be OK though? Something like this (naming to be decided):

IObservable<object> instancedBinding = BindingOperations.InstanceAsObservable(binding, myControl);

(I'm assuming you're using InstancedBinding to get an observable, let me know if you're doing something else)

[jp2masa]

It was actually new InstancedBinding and InstancedBinding.OneWay,... Anyway, it looks like I had changed that code to use a multibinding with a converter, so it shouldn't be needed for most cases. Thanks!

[YohDeadfall]

Use InstanceBinding too for making compile time bindings to a custom property system, but I need just two methods or any other way to pass ISubject<object> or IObservable<object> to Avalonia if that type will be private later. An example usage can be found here. My idea is to share the same binding setup on controls Avalonia already has, I mean using Bind method and indexers.

[dmirmilshteyn]

Will this also prevent or impact creating custom binding types? I currently use BindingBase for a very domain-specific custom compiled binding, which ends up creating an ExpressionObserver as part of the CreateExpressionObserver override.

[YohDeadfall]

Would an alternative API, be OK though? Something like this (naming to be decided):

It won't allow binding back easily. The best approach I have in mind is making a base binding class with an abstract method like that:

public abstract class BindingBase
{
    public abstract IObservable<T> ProvideValues<T>(AvaloniaObject target, AvaloniaProperty<T> targetProperty);
}

That should be enough for custom bindings without exposing too much, and WPF is about to make bindings the same except that you have no way to make expressions, but ProvideValue receives a service from which the target can be resolved. In case of a setter, there's no target object in WPF, but the target property is still present.

In the proposal above I don't like one thing .NET has, but without which it would be unusable, I mean generic virtual dispatch. It does a great job, but heck as slow, so if it's possible to avoid it, then better to do that. Therefore, the code can be slightly changes:

public abstract class Binding
{
    private protected Binding() { }
}

public abstract class Binding<T> : Binding
{
    protected Binding() { }

    public abstract IObservable<T> ProvideValues(AvaloniaObject target, AvaloniaProperty<T> targetProperty);
}

In this snippet we have two base types instead of one, but no more virtual generic dispatch. At the same time type safety is still preserved since there's a class instances of which represent bindings. No one can inherit from it except Binding<T> which has method for making an observable. There's a potential name conflict if you decide that there must be an ability to create bindings via the new operator, but it can be easily resolved by using one of two ways:

1. rename Binding<T> to BindingBase<T> and have Binding<T> and MultiBinding<T>,
2. use factory methods inside Binding to instantiate any kind of binding you want.

I like the second option mostly because it solves problem C# has, explicitly telling what type a generic parameter has. The complier cannot deduce it and won't deduce it in the foreseeable feature. There's an issue already explaining why it's hard to implement it, but in short, it would slow down the compiler a lot.

The proposal also changes the design of value converters by making them strongly typed. That's actually how Visual Studio implements them: while they inherit IValueConverter operating on objects any base converter type has strongly typed methods unboxing and boxing back values to be converted, i.e.

public abstract class ValueConverter<TFrom, TTo> : IValueConverter
{
    public abstract TTo Convert(TFrom value, object parameter, CultureInfo culture);
    public abstract TFrom ConvertBack(TTo value, object parameter, CultureInfo culture);

    // These methods do nullability checks, casts and `targetType` validation, then invoke methods above
    object IValueConverter.Convert(object value, Type targetType, object parameter, CultureInfo culture);
    object IValueConverter.ConvertBack(object value, Type targetType, object parameter, CultureInfo culture);
}

The proposal also solves #9525, but instead of an expression there's an observable to be returned or even a pair of an observer and a disposal object to kill the binding, but I would suggest going with two separate methods, one for getting an abservable, another for cleaning up the binding.

[maxkatz6]

To be kept in mind, API replacement should not use ISubject. Some optional extensions with ISubject might be added though.
For the context, we would want to drop System.Reactive dependency in the base package: #9512

[YohDeadfall]

Yeah, the proposed API actually removes that dependency at all, but if there's a need on the old behavior, when an object combines IObserver<T> and IObservable<T>, then it must be passed through two arguments or via a single one typed as IObservable<T> with internal check that it's IObserver<T> for two-way bindings.

[grokys]

Thanks for the writeup @YohDeadfall - some of that is very similar to some of the changes that we'd like to do during the 11.x timeframe, which is why I opened this issue really: to make a lot of our binding machinery internal so that we have the space to change things.

I'll take a look at reworking the public binding API with the comments in this thread mind, and also with a mind to removing any public uses of ISubject<T>.

[grokys]

Will this also prevent or impact creating custom binding types? I currently use BindingBase for a very domain-specific custom compiled binding, which ends up creating an ExpressionObserver as part of the CreateExpressionObserver override.

@dmirmilshteyn yes it definitely will impact this. Could you explain what you're doing, and what you need a little more?

[kekekeks]

There is CompiledBindingPathBuilder which is a public API since it's used by compiled XAML

[grokys]

@kekekeks regarding that API: CompiledBindingPathBuilder.Build() returns a CompiledBindingPath, which has the method public ExpressionNode BuildExpression(bool enableValidation)

ExpressionNode is one of the classes I'd like to make internal, how is CompiledBindingPathBuilder intended to be used outside of Avalonia itself? Is the method returning an ExpressionNode required as a public API?

[YohDeadfall]

@grokys, compiled bindings can produce an instance of Binding<T>, so it won't be different from custom bindings. If you want keep internals shared, I mean still used expression nodes under the hood, use source sharing as it happens in case of source generators. Yeah, it will cause some code bloating due to adding the same types to each library with compiled bindings, but it's not different from static linking.

[grokys]

WPF is about to make bindings the same except that you have no way to make expressions

@YohDeadfall is this about an upcoming change to WPF? Do they still make changes to WPF?! Do you have a link?

[YohDeadfall]

Nah, it's stale and receives fixes only as far as I know. The thing I wrote is about markup extensions in general which get a service provider containing any available info about the ambient environment which isn't limited to the target only. That's how bindings decide should they return themself (happens in setters) or expressions (happens for properties of real objects).

I learned all that stuff about ten years ago when was a WPF dev while doing localization, custom converters producing a dependency object with bindings from a template (yeah, WPF can make a DependencyObject instead of FrameworkElement, but with some help outside and some tiny binding patching when that object has no data context property).

So, from my experience I can say that WPF could be a good framework, but there's a mess with access modifiers and return types, and no flexibility at all. UWP is even worse here, it's limited as hell, but has cool ideas on deferrals and property system inherited from WPF.

It's a bit off topic, but as an improvement you can make a delayed property value materialization. For example, text editing in WPF doesn't build a new string each time you type a character. It's made only when a binding triggers or on getting a value from the property changed event args.

In case of curiosity what a markup extension receives as a service and which services are available, I recommend making a tiny app with a custom markup extension, drop a breakpoint there and get the type name in case of using it inside a setter and to bind a property on an object. There should be different set of services available. Then go to IL Spy or to WPF repo and look around.

[kekekeks]

how is CompiledBindingPathBuilder intended to be used outside of Avalonia itself?

No idea, but everything that we do from XAML is essentially a public API. Since we are referencing stuff from MSIL, we can't change it, there will be MissingMethodException otherwise.

So if there is a way to construct a binding from XAML, it should be considered to be usable from C# too.

[YohDeadfall]

In WPF the main problem with custom bindings is expressions which are private and cannot be used outside for a proper implementation, that's why I suggest observable objects all the way down, it's quite universal. So, your own and any third-party expression is an observable object which is stored by the property system. That will allow having BindingOperations working for anyone and have less abstractions.

To implement a binding in WPF you have to build your own object inherited from DependencyObject or INotifyPropertyChanged, but dependency objects are much better, and then call ProvideValue on a binding targeting that object and the property. Otherwise, BindingOperations cannot be used to observe expressions and unbound properties.

public class MyCustomBinding : MarkupExtension
{
    public override object ProvideValue(IServiceProvider serviceProvider)
    {
         // some code to check that it's not working in a setter
         // it's the same as in Binding and check the target object
         // if no target dependency object, return self

        var source = new MyCustomSource();
        var binding = new Binding { Source = myCustomSource, Path = new PropertyPath(MyCustomSource.ValueProperty) };

        // make an expression and pretend that we are the true binding
        return binding.ProvideValue(serviceProvider);
    }
}


internal class MyCustomSource : DependecyObject
{
    public static readonly DependencyProperty ValueProperty;

    // all logic goes in that object, and the property is updated
    // when a value must be transferred to the target
}

[MichalPetryka]

how is CompiledBindingPathBuilder intended to be used outside of Avalonia itself?

No idea, but everything that we do from XAML is essentially a public API. Since we are referencing stuff from MSIL, we can't change it, there will be MissingMethodException otherwise.

So if there is a way to construct a binding from XAML, it should be considered to be usable from C# too.

Having it internal and using InternalsVisibleTo or the undocumented IgnoreAccessChecksTo (doesn't work on legacy Mono) could be a temporary workaround.

[dmirmilshteyn]

@grokys This is fairly domain-specific, but really benefits from being able to somewhat define custom bindings programmatically outside of XAML.

Basically, I have a set of bindings that are convention-driven and transform a simple string into a full compiled binding path. Sometimes the resulting binding paths end up being complex and this setup allows us to insulate that complexity.

As an example, consider {MyCustomBinding Name}. The custom binding would expand Name to a full compiled binding, based on the data context (which can be one of n known data types). It would be expanded a compiled path something along the lines of {CompiledBinding x.y[Name].z}. I can also have different compiled paths generated based on other properties passed into the binding or other conventions.

This substantially simplifies development and allows a lot of flexibility when building domain-specific bindings, and it would be a shame if this feature was closed away.

[kekekeks]

Having it internal and using InternalsVisibleTo or the undocumented IgnoreAccessChecksTo (doesn't work on legacy Mono)

That still will cause MissingMethodException if we change anything and the ability to change things is the point of making them internal

[YohDeadfall]

@dmirmilshteyn, do you use reflection, observables and/or notify property changed?

I'm curious because I have a pet project which is a Reactive replacement with it's own property system. Therefore, it has custom Avalonia bindings without accessing international code like expression nodes and friends. It works even from XAML after registering a property accessor plugin. My point here is that you can use it at least as an example to solve the migration issues you might have. It doesn't support events yet, but it isn't hard.

https://github.com/YohDeadfall/Kinetic

There isn't so much documentation, so feel free to open an issue or discussion, or even write privately.

[grokys]

@dmirmilshteyn I'm currently working on this, and you should still be able to create custom binding extensions using the following pattern (in this case just binding to a predetermined property for simplicity):

    public class MyCompiledBindingExtension
    {
        public object ProvideValue(IServiceProvider serviceProvider)
        {
            var currentTimeProperty = new ClrPropertyInfo(
                "CurrentTime",
                x => ((MainWindowViewModel)x).CurrentTime,
                null,
                typeof(string));

            var builder = new CompiledBindingPathBuilder()
                .Property(currentTimeProperty, PropertyInfoAccessorFactory.CreateInpcPropertyAccessor);

            return new CompiledBindingExtension(builder.Build());
        }
    }

Does this work for you?

[dmirmilshteyn]

@grokys Yeah, that would be perfect!