CommunityToolkit / dotnet

.NET Community Toolkit is a collection of helpers and APIs that work for all .NET developers and are agnostic of any specific UI platform. The toolkit is maintained and published by Microsoft, and part of the .NET Foundation.
https://docs.microsoft.com/dotnet/communitytoolkit/?WT.mc_id=dotnet-0000-bramin
Other
2.99k stars 294 forks source link

Cannot override Broadcast when using ObservableRecipient attribute #728

Open echoix opened 1 year ago

echoix commented 1 year ago

Describe the bug

Documentation of ObservableRecipient.Broadcast indicate that Broadcast(...) should be overridden in order to use a custom channel (token). If we need to use the features of ObservableValidator and need to use [NotifyPropertyChangedRecipients], then we need to use [ObservableRecipient] on the class in order to have the missing pieces. I couldn't read online the documentation of the ObservableRecipient.Broadcast method generated by the attribute since the API docs for this dotnet CommunityToolkit aren't published on Microsoft Learn, only the old Windows Community Toolkit 7.0.0 are available (https://learn.microsoft.com/en-us/dotnet/api/communitytoolkit.mvvm.componentmodel.observablerecipient?view=win-comm-toolkit-dotnet-7.0) (and it didn't exist). So I referred to the XML docs in the SourceLink using F12 in Visual Studio.

Regression

No response

Steps to reproduce

I made a new empty WPF project, and created a little sample pointing out what works and also the problem when wanting to add ObservableValidator in the mix.

using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Messaging;
using CommunityToolkit.Mvvm.Messaging.Messages;

using System;
using System.ComponentModel.DataAnnotations;

namespace WpfApp3
{
    public partial class MyFirstObservableClass : ObservableRecipient
    {
        [ObservableProperty]
        [NotifyPropertyChangedRecipients]
        private int myFirstIntProperty;

        [ObservableProperty]
        [NotifyPropertyChangedRecipients]
        private int myFirstIntProperty2;

        protected override void Broadcast<T>(T oldValue, T newValue, string? propertyName)
        {
            var message = new PropertyChangedMessage<T>(this, propertyName, oldValue, newValue);
            _ = Messenger.Send(message, $"{nameof(MyFirstObservableClass)}.{propertyName}");
        }
    }

    [ObservableRecipient]
    public partial class MySecondObservableClass : ObservableValidator
    {
        [ObservableProperty]
        [Range(1, 21)]
        [NotifyPropertyChangedRecipients]
        private int mySecondIntProperty;

        //// Severity   Code    Description Project File    Line    Suppression State
        //// Error CS0111  Type 'MySecondObservableClass' already defines a member called 'Broadcast' with the same parameter types WpfApp3 CommunityToolkit.Mvvm.SourceGenerators\CommunityToolkit.Mvvm.SourceGenerators.ObservableRecipientGenerator\WpfApp3.MySecondObservableClass.g.cs 111 Active
        //protected virtual void Broadcast<T>(T oldValue, T newValue, string? propertyName)
        //{
        //    var message = new PropertyChangedMessage<T>(this, propertyName, oldValue, newValue);
        //    _ = Messenger.Send(message, nameof(MySecondIntProperty));
        //}

        //// Same error CS0111 at build, but also the following IntelliSense when writing:
        ////
        //// Severity   Code    Description Project File    Line    Suppression State
        //// Error CS0115   'MySecondObservableClass.Broadcast<T>(T, T, string?)': no suitable method found to override WpfApp3 C:\Users\<hidden>\WpfApp3\WpfApp3\MyObservableClass.cs  49  Active
        //protected override void Broadcast<T>(T oldValue, T newValue, string? propertyName)
        //{
        //    var message = new PropertyChangedMessage<T>(this, propertyName, oldValue, newValue);
        //    _ = Messenger.Send(message, nameof(MySecondIntProperty));
        //}
    }

    public partial class MyFirstRecipient
        : ObservableRecipient,
            IRecipient<PropertyChangedMessage<int>>
    {
        public MyFirstRecipient()
        {
            IsActive = true;
        }

        public void Receive(PropertyChangedMessage<int> message)
        {
            throw new NotImplementedException();
        }
    }

    public partial class MySecondRecipient
        : ObservableRecipient,
            IRecipient<PropertyChangedMessage<int>>
    {
        public MySecondRecipient()
        {
            IsActive = true;
        }

        protected override void OnActivated()
        {
            Messenger.RegisterAll<string>(
                this,
                $"{nameof(MyFirstObservableClass)}.{nameof(MyFirstObservableClass.MyFirstIntProperty2)}"
            );
        }

        public void Receive(PropertyChangedMessage<int> message)
        {
            throw new NotImplementedException();
        }
    }

    public partial class MyThirdRecipient : ObservableRecipient
    {
        [ObservableProperty]
        private int fromMyFirstIntProperty;

        public MyThirdRecipient()
        {
            IsActive = true;
        }

        protected override void OnActivated()
        {
            // Using a method group...
            Messenger.Register<MyThirdRecipient, PropertyChangedMessage<int>, string>(
                this,
                $"{nameof(MyFirstObservableClass)}.{nameof(MyFirstObservableClass.MyFirstIntProperty)}",
                (r, m) => r.Receive1(m)
            );
            Messenger.Register<MyThirdRecipient, PropertyChangedMessage<int>, string>(
                this,
                $"{nameof(MyFirstObservableClass)}.{nameof(MyFirstObservableClass.MyFirstIntProperty2)}",
                (r, m) => r.Receive2(m)
            );

            base.OnActivated();
        }

        public void Receive(PropertyChangedMessage<int> message)
        {
            throw new NotImplementedException();
        }

        public void Receive1(PropertyChangedMessage<int> message)
        {
            if (
                message.Sender.GetType() == typeof(MyFirstObservableClass)
                && message.PropertyName == nameof(MyFirstObservableClass.MyFirstIntProperty)
            )
            {
                FromMyFirstIntProperty = message.NewValue;
            }
        }

        public void Receive2(PropertyChangedMessage<int> message)
        {
            if (
                message.Sender.GetType() == typeof(MyFirstObservableClass)
                && message.PropertyName == nameof(MyFirstObservableClass.MyFirstIntProperty2)
            )
            {
                FromMyFirstIntProperty = message.NewValue;
            }
        }
    }
}

Expected behavior

Be able to use the token feature with [NotifyPropertyChangedRecipients] when using an ObservableValidator class, thus I must be able to override the Broadcast method.

Screenshots

No response

IDE and version

VS 2022

IDE version

VisualStudio.17.Release/17.6.4+33815.320

Nuget packages

Nuget package version(s)

8.2.0

Additional context

In the source generator ObservableRecipientGenerator.cs, there is already some logic to not generate some methods if it already exists. https://github.com/CommunityToolkit/dotnet/blob/e071ed22b379f904820475db827be3ad04d3e96e/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/ObservableRecipientGenerator.cs

What I would see as a solution would be to have the same kind of checks, for other methods, like Broadcast, and we would probably see this file have a new field in the record: https://github.com/CommunityToolkit/dotnet/blob/e071ed22b379f904820475db827be3ad04d3e96e/src/CommunityToolkit.Mvvm.SourceGenerators/ComponentModel/Models/ObservableRecipientInfo.cs

Help us help you

No, just wanted to report this

echoix commented 1 year ago

I'd like to know if this issue was acknowledged and if there was any missing information for this bug, or any other blockers I should be aware of.

JochenMader commented 10 months ago

Hi @echoix

It took me a while to understand the problem but finaly I go it.

When the ObservableRecipientAttribute is used, the Broadcast method is generated in the partial class itself. So it's not possible to override it directly for custom behavior. The generated method has following comment:

    /// <remarks>
    /// You should override this method if you wish to customize the channel being
    /// used to send the message (eg. if you need to use a specific token for the channel).
    /// </remarks>

So, if you want ObservableValidator and have custom behavior of the Broadcast, just give it an overridable class:

[ObservableRecipient]
public partial class MySecondObservableClassRecipient : ObservableValidator { }
public partial class MySecondObservableClass : MySecondObservableClassRecipient
{
    [ObservableProperty]
    [Range(1, 21)]
    [NotifyPropertyChangedRecipients]
    private int mySecondIntProperty;

    protected override void Broadcast<T>(T oldValue, T newValue, string? propertyName)
    {
        // my broadcast customization
        base.Broadcast(oldValue, newValue, propertyName);
    }
}

best regards