Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] WhenAny[Value] does not fire on DependencyProperty if INotifyPropertyChanged is implemented #3111

Open
Renaudyes opened this issue Dec 15, 2021 · 4 comments
Labels

Comments

@Renaudyes
Copy link

Describe the bug
I am trying to use this.WhenAnyValue to observe changes to a custom DependencyProperty of a UserControl.

  • If the class that derives from the UserControl does not implement INotifyPropertyChanged, this.WhenAnyValue fires as forseen.
  • If the class that derives from the UserControl does implement INotifyPropertyChanged, this.WhenAnyValue fires once and never after.

Step to reproduce

  1. Create the following classes :
public class CustomControlThatWorks : ContentControl
    {
        public int Value
        {
            get { return (int)GetValue(ValueProperty); }
            set { SetValue(ValueProperty, value); }
        }

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

        public CustomContentControlTest()
        {
            _ = Task.Run(async () =>
            {
                while(true)
                {
                    await Task.Delay(2000);
                    System.Windows.Application.Current.Dispatcher.Invoke(() =>
                    {
                        Value = Value + 1;
                    });

                }
            });
        }
    }
public class CustomControlThatDoesNotWork : ContentControl, INotifyPropertyChanged
    {
        public int Value
        {
            get { return (int)GetValue(ValueProperty); }
            set { SetValue(ValueProperty, value); }
        }

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

        public event PropertyChangedEventHandler PropertyChanged;

        public CustomContentControlTest()
        {
            _ = Task.Run(async () =>
            {
                while(true)
                {
                    await Task.Delay(2000);
                    System.Windows.Application.Current.Dispatcher.Invoke(() =>
                    {
                        Value = Value + 1;
                    });

                }
            });
        }

    }

These two classes only differ by the INotifyPropertyChanged . They will change the DependencyProperty Value every 2s.

  1. Now create a UserControl named "UserControlTest" that will use both of those classes
    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition Height="*"></RowDefinition>
            <RowDefinition Height="*"></RowDefinition>
            <RowDefinition Height="*"></RowDefinition>
            <RowDefinition Height="*"></RowDefinition>
        </Grid.RowDefinitions>
        <local:CustomControlThatWorks  x:Name="custom1" Grid.Row="0"></local:CustomControlThatWorks>
        <local:CustomControlThatDoesNotWork   x:Name="custom2" Grid.Row="1"></local:CustomControlThatDoesNotWork >
        <TextBlock x:Name="textblock1" Grid.Row="2" Text="{Binding ElementName=custom1, Path=Value}"></TextBlock>
        <TextBlock x:Name="textblock2" Grid.Row="3" Text="{Binding ElementName=custom2, Path=Value}"></TextBlock>
  </Grid>
  1. Go to the UserControlTest.xaml.cs
public partial class UserControlTest: UserControl
{
        public UserControlTest()
        {
             this.WhenActivated(d =>
            {
                this.WhenAnyValue(x => x.custom1.Value).Subscribe(x => Console.WriteLine(x)).DisposeWith(d); // will fire every 2s
                this.WhenAnyValue(x => x.custom2.Value).Subscribe(x => Console.WriteLine(x)).DisposeWith(d); // will only fire once
                this.WhenAnyValue(x => x.textblock1.Text).Subscribe(x => Console.WriteLine(x)).DisposeWith(d); // will fire every 2s
                this.WhenAnyValue(x => x.textblock2.Text).Subscribe(x => Console.WriteLine(x)).DisposeWith(d); // will fire every 2s
            }
        }
}
  1. You should see that the TextBlock textblock1 and textblock2 will update their "Text" property because the DependencProperty is well set.

Expected behaviour
This line this.WhenAnyValue(x => x.custom2.Value).Subscribe(x => Console.WriteLine(x)).DisposeWith(d); should fire every 2s as the others.

Environment

  • OS: Windows 10 x64
  • Version .NET5
  • ReactiveUI Version: 16.2.6
  • ReactiveUI.WPF 16.2.6
@Renaudyes Renaudyes added the bug label Dec 15, 2021
@niklas-holma
Copy link

Interesting.

If i were to modify the CustomControlThatDoesNotWork with the following:

        public int Value
        {
            get { return (int)GetValue(ValueProperty); }
            set
            {
                SetValue(ValueProperty, value);
                PropertyChanged?.Invoke(nameof(Value));
            }
        }

Would you expect it to fire twice for every set?

@Renaudyes
Copy link
Author

The problem is you are changing the behavior of a dependency property. You should know that the SetValue(ValueProperty, value) can directly used by the WPF Engine for performance reason. And then never call the setter of your property Value.

It will work for most cases but does not handle the problem correctly and will not for 100% of the cases.

@glennawatson
Copy link
Contributor

glennawatson commented Dec 22, 2021

This is deliberate functionality on our parts but can be overridden.

The INPC votes 5 that it can handle the property changed

return target.GetTypeInfo().IsAssignableFrom(type.GetTypeInfo()) ? 5 : 0;
while dependency property votes 4
return GetDependencyProperty(type, propertyName) is not null ? 4 : 0;

You could make your own https://github.com/reactiveui/ReactiveUI/blob/00b48d37f49022e49fd105bb6a1d737f1c237c2b/src/ReactiveUI.Wpf/DependencyObjectObservableForProperty.cs and register with Splat.

Splat.Locator.CurrentMutable.Register(typeof(ICreatesObservableForProperty), () => new MyDepedencyObjectObservablePropertyCreator())

I might make our version have a virtual method on the voting to allow you to derive.

GitHub
An advanced, composable, functional reactive model-view-viewmodel framework for all .NET platforms that is inspired by functional reactive programming. ReactiveUI allows you to abstract mutable st...

@niklas-holma
Copy link

The problem is you are changing the behavior of a dependency property. You should know that the SetValue(ValueProperty, value) can directly used by the WPF Engine for performance reason. And then never call the setter of your property Value.

It will work for most cases but does not handle the problem correctly and will not for 100% of the cases.

No sorry. I didnt mean to suggest it was a solution to your problem.

I was trying to hint that DependencyObjects and INotifyPropertyChanged use two different mechanisms for providing property change events. I would really try to avoid to combine a View Model and a Control in one object (MVVM separates theese).

Value = Value + 1;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Value)));

In the above code, a WPF binding would only hit on the first line, and ignore the second line since it checks for DependencyProperty first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants