Events inheritance is not supported by EventHandlers

[korzonkiee]

I'm submitting a...

[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Given an abstract class (say Mammal) and its derived classes (say Dolphin and Monkey), the event handler defined as @EventHandler(Mammal) does not invoke its handle method when Dolphin or Monkey event is published to the event bus.

Expected behavior

Event handlers defined for abstract classes should be invoked when an instance of a derived class is published onto the event bus.

Minimal reproduction of the problem with instructions

1. git clone https://github.com/korzonkiee/nestjs-eventhandlers
2. cd nestjs-eventhandlers
3. npm install
4. npm run start:dev
5. open http://localhost:3000/dolphin in your browser.
6. mammal should be logged into the console by the src/mammal.handler.ts, but is not.

What is the motivation / use case for changing the behavior?

In my system, I have multiple events that are related to Task, (TaskCreated, TaskExecuted, etc...). Each of those events extends an abstract class TaskEvent. I would like to create an event handler that handles all of the events that extend TaskEvent without manually listing them in the event handler's decorator constructor.

Additional info

I looked through the code responsible for relaying events into a specific event handler and it turned out that upcoming events are compared with a list of events specified in the decorator by name:

https://github.com/nestjs/cqrs/blob/master/src/event-bus.ts#L104.

So, when we define @EventsHandler(Mammal) and publish an event Dolphin, it compares "Mammal" === "Dolphin" hence not dispatching the event into the event handler.

I've changed the source code slightly so that it now compares the events using an instanceof operator and it works as expected, but I'm not sure if that is the desired solution.

Environment

Nest version: 7.1.1
Nest CQRS version: 7.0.1


For Tooling issues:
- Node version: 14.14.0
- Platform:  Mac

Others:

[kamilmysliwiec]

Would you like to create a PR for this issue?

[korzonkiee]

Sure

[kodeine]

@korzonkiee were you able to submit the PR for this?

[korzonkiee]

@kodeine not yet. IIRC I had some TypeScript issues when trying to implement this functionality. I might give it another try soon.

[peixotoleonardo]

I believe it's not a bug. To invoke its handle method when Dolphin or Monkey you should add this events in @EventsHandler

import { EventsHandler } from '@nestjs/cqrs';
import { IEventHandler } from '@nestjs/cqrs';
import { Dolphin, Mammal, Monkey } from './animal';

@EventsHandler(Mammal, Dolphin, Monkey)
export class MammalHandler implements IEventHandler<Mammal> {
    handle(event: Mammal) {
        console.log(`mammal`);
    }
}

[Sikora00]

I heard about an approach that every Event is extended per producer. Let say we have an event called TurnomentWonEvent which can be produced by FinishEnemyCommand and SurrenderCommand and we can create TurnomentWonBySurrenderEvent and so on. All of those should obviously trigger TurnomentWonEventHandler. You can ask why do we need this? We can discuss if it is a good practice to do inheritance with an intention to extend event handlers. Maybe using it this way is an antipattern, but the approach described above definitely improves debugging. We can make it really clean what produced the event that can be really hard to figure out, we can also add some specific data from the command to the custom event and Log all events from Saga if we want. Now I think that basically, we can add some additional data to all events that can describe the producer so we don't need inheritance and maybe this is better to not encourage people to use it :D