[BUG] WPF: BindCommand cause warning in log for button declared in XAML

[mgnslndh]

Describe the bug

If I use BindCommand to bind a view model command to a button declared in XAML like this:

this.BindCommand(ViewModel, vm => vm.DecreaseSpeed, view => view.DecreaseSpeed);

I will then get a warning in the log like this:

POCOObservableForProperty: The class ...PreviewView property DecreaseSpeed is a POCO type and won't send change notifications, WhenAny will only return a single value!

The warning is wrong since the button DecreaseSpeed is not a property on the view but a field, declared like this in XAML:

<RepeatButton Name="DecreaseSpeed" Content="Decrease"/>

Steps To Reproduce

1. Create a view model with single command property
2. Create a corresponding view to the view model and implement IViewFor
3. Declare a named button in view XAML
4. Add call to BindCommand in view constructor to bind the view model command to the button
5. When the view model is assigned to the view the BindCommand will execute and the warning will appear in the log

I'm working on a test case to show this.

Expected behavior
I expect not to see any warnings in the log when doing something expected by the library, like binding a command to a button declared in XAML.

Environment

• OS: Windows 10
• Version: 9.12.0 to 9.15.4
• Device: PC

Additional context

I have chatted with @glennawatson on slack about this issue https://reactivex.slack.com/archives/C02AJB872/p1558086054213400

[open-collective-bot]

Hey @mgnslndh 👋,

Thank you for opening an issue. We will get back to you as soon as we can. Also, check out our Open Collective and consider backing us.

https://opencollective.com/reactiveui

PS.: We offer priority support for all backers. Don't forget to add priority label when you start backing us 😄

ReactiveUI - Open Collective

An advanced, composable, functional reactive model-view-viewmodel framework for all .NET platforms!

[glennawatson]

As discussed we probably want to suppress the message in the case of WPF BindCommand on the view property. We still want to attempt using RxUI binding on both sides since there may be more unusual situations when both properties are notifiable.

[mgnslndh]

The following code in ReactiveNotifyPropertyChangedMixin.cs will try to find an appropriate implementation of ICreatesObservableForProperty but in my case DecreaseSpeed is not a property but a field.

private static IObservable<IObservedChange<object, object>> NotifyForProperty(object sender, Expression expression, bool beforeChange)
{
    var propertyName = expression.GetMemberInfo().Name;
    var result = notifyFactoryCache.Get(Tuple.Create(sender.GetType(), propertyName, beforeChange));

    if (result == null)
    {
        throw new Exception($"Could not find a ICreatesObservableForProperty for {sender.GetType()} property {propertyName}. This should never happen, your service locator is probably broken. Please make sure you have installed the latest version of the ReactiveUI packages for your platform. See https://reactiveui.net/docs/getting-started/installation/nuget-packages for guidance.");
    }

    return result.GetNotificationForProperty(sender, expression, propertyName, beforeChange);
}

Since there is no appropriate implementation the POCOObservableForProperty will be selected and it will naturally log the warning.

I'm not 100% sure I understand all the code but either ReactiveUI should see that it is a FieldExpression and not try to resolve an ICreatesObservableForProperty but I suspect it is needed to get at least the "first" value. Or, ReactiveUI could have a specific implementation of ICreatesObservableForProperty for fields (a DependencyObject with both Command and CommandParameter properties) declared in views.

[mgnslndh]

How would POCOObservableForProperty check to find out if it should suppress the message? It does not know it executes in the context of a BindCommand call, I think.

[mgnslndh]

I have a test case in my local repo for this issue and I'm willing to do a PR but I need guidance on the best approach of fixing this. Thanks for helping out @glennawatson!

[glennawatson]

ICreatesObservableForProperty.GetNotificationForProperty

Add the ability to suppress parameters by adding a new parameter with a default value of false

I would change the following

ReactiveUI/src/ReactiveUI/Interfaces/ICreatesObservableForProperty.cs

Line 50 in eb53eee

IObservable<IObservedChange<object, object>> GetNotificationForProperty(object sender, Expression expression, string propertyName, bool beforeChanged = false);

to:

IObservable<IObservedChange<object, object>> GetNotificationForProperty(object sender, Expression expression, string propertyName, bool beforeChanged = false, bool supressWarnings = false);

ReactiveNotifyPropertyChangedMixin.NotifyForProperty

ReactiveUI/src/ReactiveUI/Mixins/ReactiveNotifyPropertyChangedMixin.cs

Line 199 in ef471c0

private static IObservable<IObservedChange<object, object>> NotifyForProperty(object sender, Expression expression, bool beforeChange)

I would add the parameter bool suppressMessage and pass that into GetNotificationForProperty

ReactiveNotifyPropertyChangedMixin.NestedObservedChanges

ReactiveUI/src/ReactiveUI/Mixins/ReactiveNotifyPropertyChangedMixin.cs

Line 182 in ef471c0

private static IObservable<IObservedChange<object, object>> NestedObservedChanges(Expression expression, IObservedChange<object, object> sourceChange, bool beforeChange)

I would add a parameter bool suppressMessage = false, and pass it into NotifyForProperty

ReactiveNotifyPropertyChangedMixin.SubscribeToExpressionChain

ReactiveUI/src/ReactiveUI/Mixins/ReactiveNotifyPropertyChangedMixin.cs

Line 132 in ef471c0

public static IObservable<IObservedChange<TSender, TValue>> SubscribeToExpressionChain<TSender, TValue>(

I would add the parameter bool suppressMessage = false, and pass into NestedObservedChanges

RxApp

I would add a public property called SuppressViewCommandBindingMessage

ReactiveUI.Wpf.Registrations

Set RxApp.SuppressViewCommandBindingMessage = true

CommandBinderImplementation.BindCommandInternal

ReactiveUI/src/ReactiveUI/Bindings/Command/CommandBinderImplementation.cs

Line 141 in ddb56c7

view.WhenAnyDynamic(controlExpression, x => x.Value),

This is where you will pass in the "suppressWarnings" as true for WPF.

You will need to change the line

from:

view.WhenAnyDynamic(controlExpression, x => x.Value),

to:

view.SubscribeToExpressionChain(controlExpression, false, false, RxApp.SuppressViewCommandBindingMessage).Select(x => x.Value),

Summary

The changes will pass down a optional suppress message chain downwards. We use RxApp to indicate if we suppress or not.