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

Overhaul events to use EventHandler #132

Merged
merged 8 commits into from
Oct 26, 2023
Merged

Overhaul events to use EventHandler #132

merged 8 commits into from
Oct 26, 2023

Conversation

gigajuwels
Copy link
Contributor

This PR deprecates the old style source.On(eventId, ...) style event system, with the standard C# EventHandler system. The old event system is still partially supported (but will be removed).

The only events that remain using the EventEmitter.NET system are events that have a dynamic eventId or a generic type as the event parameter

@skibitsky
Copy link
Member

Thanks! This PR is very welcomed! However, I'd suggest to completely remove EventEmitter.NET to have consistent approach towards events and avoid external dependency.

@gigajuwels
Copy link
Contributor Author

We can remove the obsolete events, however we will still need EventEmitter.NET to execute the remaining dynamic events. See here and here for examples where a normal EventHandler would not suffice. A dynamic event is an event that

  1. Has a dynamic event id / name. The event id/name is not known at compile time, but only known at runtime (i.e a request/response method, here)
  2. Has a generic argument type T. Example can be seen here

The EventEmitter.NET library is the now removed WalletConnectSharp.Events library. This kind of library is better placed as its own thing outside WalletConnect, so it does not need to manage this kind of complex dynamic dispatch logic.

@skibitsky skibitsky self-requested a review October 20, 2023 04:59
gigajuwels and others added 3 commits October 24, 2023 21:32
# Conflicts:
#	Tests/WalletConnectSharp.Network.Tests/RelayTests.cs
#	WalletConnectSharp.Core/Controllers/Relayer.cs
#	WalletConnectSharp.Core/Controllers/Subscriber.cs
#	WalletConnectSharp.Core/WalletConnectCore.cs
@skibitsky skibitsky merged commit 2dbfd51 into 2.0 Oct 26, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants