ryardley / ts-bus

A lightweight JavaScript/TypeScript event bus to help manage your application architecture.
MIT License
134 stars 8 forks source link

Multiple subscriptions on eventemitter #47

Open ryardley opened 4 years ago

ryardley commented 4 years ago

When using the array syntax for subscribers the code currently creates a new global subscription for every predicate provided.

// Creates two subscriptions on "**"
bus.subscribe([somePredicate, someOtherPredicate], (event) => {
  console.log(event.type);
});

We need a way to only have a single predicate subscription but alter the handler to include multiple predicates.

dmicic commented 4 years ago

Just to reflect our discussion over here as well.

The problem applies to array-based subscriptions in general and not only to predicates. In the current implementation, for a published message, the bus would call the handler function for each matched event definition passed in the array argument.

The bus must ensure that the handler function gets called exactly once for an event that matches 1 to n defined event subscriptions.

dmicic commented 4 years ago

Once implemented, the test here should be amended and moved to the index.ts. Or simply deleted if it becomes redundant.

ryardley commented 4 years ago

How far do we go though?

const handler = (event) => {
  // do stuff
};

function somePredicate(e) {
  return e.type.indexOf('test.') === 0;
}

bus.subscribe([somePredicate, someOtherPredicate], handler);
bus.subscribe('test.*', handler);

bus.publish({type:'test.foo'});

What is the expected behaviour? If we use eventemitter2 with the wildcard syntax the way it works currently even if we implement something fancy for managing predicates we will have two handlers get registered and two handlers fired no matter what.

Even with this:

bus.subscribe([somePredicate, someOtherPredicate,'test.*'], handler);

I am starting to favour replacing EventEmitter2 with a custom event emitter that is run on filters to manage listeners.

That way we can write a function that creates a predicate based on the wildcard syntax eg:

parseWildCard("test.*")({ type:"test.foo" }); // true

and then we can create a filtering system based on predicates.

dmicic commented 4 years ago

Yes, exactly! As long as we stay with eventemitter2, we will always end up with 2 handlers been fired (if we mix predicate mit non-predicate based events) as here:

bus.subscribe([somePredicate, someOtherPredicate,'test.*'], handler);

Hence, It doesn't make much sense to build a abstraction layer over it, the result will be the same and then we are as far as I was in my PR where event defintions were split in 2 arrays.

In short, it's probably a great opportunity to introduce a simple bespoke event emitter.

And regarding this code here:

bus.subscribe([somePredicate, someOtherPredicate], handler);
bus.subscribe('test.*', handler);

I would be fine if in that case the handler gets called twice. I don't see a reason why somebody would have to use the bus this way. Having said that if we can address that issue and call the handler only once it would "probably" be better. This would have to be document well as both behaviors somehow could be misleading :)

In regards to the wildcard subscription. Yes, creating a predicate makes absolutely sense so we don't have to deal with non-predicate subscriptions under the hood anymore. I was just thinking whether it would actually make sense to introduce regex based matching instead of wildcards?