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
3.04k stars 298 forks source link

Add WeakEventManager/WeakEventListener #404

Open GalaxiaGuy opened 2 years ago

GalaxiaGuy commented 2 years ago

Overview

A class for weakly subscribing to events is a useful concept that has been implemented many times in various projects.

I currently use Xamarin Forms with Xamarin Community Toolkit, and rely on the ObservableObject implementation that uses WeakEventManager for INotifyPropertyChanged.PropertyChanged.

Maui Community Toolkit however does not have its own ObservableObject, instead directing people to use the one here (an idea I fully support). However the one here does not support weak subscribing (because there is no WeakEventManager).

I propose adding WeakEventManager - probably the one one currently implemented in Maui (https://github.com/dotnet/maui/blob/aa2d11137e8b3226d85a39da4f605bf26ab42aa0/src/Core/src/WeakEventManager.cs), and then adding a way for ObservableObejct to (optionally) use it.

(I have a separate issue for that: https://github.com/CommunityToolkit/dotnet/issues/80)

API breakdown

(Copied from Maui)

public class WeakEventManager
{
    public void AddEventHandler<TEventArgs>(EventHandler<TEventArgs> handler, [CallerMemberName] string eventName = "")
        where TEventArgs : EventArgs
    {
    }

    public void AddEventHandler(Delegate? handler, [CallerMemberName] string eventName = "")
    {
    }

    public void HandleEvent(object sender, object args, string eventName)
    {
    }

    public void RemoveEventHandler<TEventArgs>(EventHandler<TEventArgs> handler, [CallerMemberName] string eventName = "")
        where TEventArgs : EventArgs
    {
    }

    public void RemoveEventHandler(Delegate? handler, [CallerMemberName] string eventName = "")
    {
    }
}

Usage example

public event PropertyChangedEventHandler? PropertyChanged
{
    add => _weakEventManager.AddEventHandler(value);
    remove => _weakEventManager.RemoveEventHandler(value);
}

Breaking change?

No

Alternatives

There is a proposal to just add this to the BCL: https://github.com/dotnet/runtime/issues/61517

Additional context

No response

Help us help you

No, just wanted to propose this.

If using the Maui one unchanged is desirable, I'd be able to make such a PR.

michael-hawker commented 1 year ago

Was looking a bit more into this, as I think it'd make sense to not port the Windows Community Toolkit one either forward but instead add that (or similar code here).

Looking at the MAUI one, I believe they both service different purposes, so there may be a need for two different APIs/helpers here.

  1. On one side, like the MAUI one, it's at the implementation level of the event itself and someone owning that code, such that folks subscribing to the event don't capture the reference to the instance.
  2. On the other side, like the WCT one, it's at the reference level of someone wanting to listen to the event without capturing the instance. This is useful for classes in from the framework where the code isn't owned.

So, I think both patterns are required for different scenarios?

michael-hawker commented 1 year ago

Was just wondering if we could improve upon the implementation from the WCT and simplify its usage:

var inpc = rowGroupInfo.CollectionViewGroup as INotifyPropertyChanged;
var weakPropertyChangedListener = new WeakEventListener<DataGrid, object, PropertyChangedEventArgs>(this) {
    OnEventAction = static (instance, source, eventArgs) => instance.CollectionViewGroup_PropertyChanged(source, eventArgs),
    OnDetachAction = (weakEventListener) => inpc.PropertyChanged -= weakEventListener.OnEvent // Use Local References Only
}
inpc.PropertyChanged += weakPropertyChangedListener.OnEvent

to this:

rowGroupInfo.CollectionViewGroup.PropertyChanged +=
    (new WeakEventListener<DataGrid, object, PropertyChangedEventArgs>(this) {
        OnEventAction = static (@this, source, eventArgs) => @this.CollectionViewGroup_PropertyChanged(source, eventArgs),
        OnDetachAction = static (source, listener) => source.PropertyChanged -= listener.OnEvent
    }).OnEvent;

Basically add the source as a parameter on the OnDetachAction so it can be static as well and remove the possibility of accidently capturing a local reference or the reference to the instance holding the event?

(Or is this still going to capture the source reference for the detach?)