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

Add WeakEventManager/WeakEventListener #404

Open
GalaxiaGuy opened this issue Aug 26, 2022 · 2 comments
Open

Add WeakEventManager/WeakEventListener #404

GalaxiaGuy opened this issue Aug 26, 2022 · 2 comments
Labels
feature request 📬 A request for new changes to improve functionality

Comments

@GalaxiaGuy
Copy link

GalaxiaGuy commented Aug 26, 2022

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: #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:
dotnet/runtime#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.

@GalaxiaGuy GalaxiaGuy added the feature request 📬 A request for new changes to improve functionality label Aug 26, 2022
@michael-hawker michael-hawker changed the title Add WeakEventManager Add WeakEventManager/WeakEventListener Mar 24, 2023
@michael-hawker
Copy link
Member

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
Copy link
Member

michael-hawker commented Apr 6, 2023

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?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request 📬 A request for new changes to improve functionality
Projects
None yet
Development

No branches or pull requests

2 participants