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]: IMessageBus Publish overload extension method not called #772

Closed
9 tasks done
GuidoNeele opened this issue Oct 27, 2021 · 4 comments
Closed
9 tasks done

[Bug]: IMessageBus Publish overload extension method not called #772

GuidoNeele opened this issue Oct 27, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@GuidoNeele
Copy link

GuidoNeele commented Oct 27, 2021

Component

Core (Settings, Startup)

What platform(s) are effected?

  • iOS 12
  • iOS 13
  • iOS 14
  • Android 8
  • Android 9
  • Android 10
  • Android 11
  • UWP (Sponsors ONLY)

Steps To Reproduce

  1. Create MessageBus listener var listener = MessageBus.Listener("EventName").Subscribe(DoWork)
  2. Publish event name only MessageBus.Publish("EventName")

The DoWork method will not be called when "EventName" is published.

Expected Behavior

When calling MessageBus.Publish("EventName") the Subscribe method should be called on Listeners with the same event name. This behavior only happens when the event names are equal and no other values are being passed.

Actual Behavior

The Publish(object message) method on MessageBus is called directly without going through the extension method Publish(this IMessageBus msgBus, string name). This is because methods with matching parameters on a class are executed before extension methods.

Publish("EventName") matches public void Publish(object message)

public void Publish(object message)
{
if (this.messageSinks.TryGetValue(message.GetType(), out var subj))
subj.OnNext(message);
}

public static void Publish(this IMessageBus msgBus, string name)
=> msgBus.Publish(new NamedMessage<Unit>(name, Unit.Default));

Exception or Log output

No exception

Code Sample

See https://dotnetfiddle.net/6knsV6 for an example of this behavior.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@GuidoNeele GuidoNeele added bug Something isn't working unverified This issue has not been verified by a maintainer labels Oct 27, 2021
@aritchie
Copy link
Member

The message bus wasn't really meant for public consumption and I don't believe I'm using named messages internally. Are you using this somewhere? You should know it was marked for death since it does create a couple of hotspots internally that slow things down due to locking - namely beacons & http transfers.

@GuidoNeele
Copy link
Author

Yes I'm using it for a few messages, not extensively. Most times I'm sending some data with it and then it works fine. Now I wanted to use it without data, just as a notification, and ran into this issue. So I'm better of using Xamarin.Forms.MessagingCenter?

@aritchie
Copy link
Member

No - I'll keep it if it is being used. It isn't a big chunk of code. You are definitely better off with the shiny message bus vs xf messaging center lol. The fix wasn't hard either - I just had to make sure nothing else was using those signatures.

It will likely just be removed from being used in the internals of Shiny.

@GuidoNeele
Copy link
Author

That's exactly why I'm using IMessageBus instead of Xamarin.Forms.MessagingCenter ;-) Saw the fix, so I'll wait till the build is available and then use the renamed methods. Thanks for the quick fix!

@aritchie aritchie removed the unverified This issue has not been verified by a maintainer label Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants