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

feat: add generic parameter for IEventPublisher in EventBus #1930

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lucas-gregoire
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Currently, the EventBus.publish and EventBus.publishAll methods always return any, whereas they could infer types from the publish and publishAll methods of the IEventPublisher interface.

What is the new behavior?

This PR introduces a new generic parameter for the EventBus which represents the type of the event publisher (extending IEventPublisher), allowing to infer the return types of the publish and publishAll methods.

If the event publisher does not have a publishAll method—like in the case of DefaultPubSub—then the return type of publishAll is an array of the publish results from the publisher. This clarifies the behavior of the publishAll method.

By default, publish still returns any while publishAll now returns any[] which better matches the returned array.

Does this PR introduce a breaking change?

  • Yes : publishAll now returns any[] instead of any by default. I think this is a "soft" breaking change.
  • No

For this breaking change, here are the situations to consider:

  • Users calling publishAll without using its result: no change.
  • Users using the result of publishAll:
    • With the default event publisher or a custom publisher without a publishAll method:
      • Casting it to an array of something: this cast will still compile.
      • Casting it to something other than an array: this cast will no longer compile. While this situation doesn't make much sense, it is still possible. Users will have to adjust their code.
    • With a custom event publisher that implements a publishAll method:
      • Casting it to the result of publishAll: this cast will still compile but could be replaced by the inference allowed by the new generic parameter.

Personally, I think this is a breaking change with not much impact.

Other information

I took the opportunity of this PR to add the expect-type library to more easily test types and generics. I selected this lib among many others (tsd, ts-expect, type-testing, tstyche) because this one is still maintained, used by many projects, and relatively comprehensive. If you have other suggestions, feel free to share them.

I also added some missing generics tests on query handlers.

@lucas-gregoire lucas-gregoire force-pushed the feat/type-checking-event-bus branch from 04605e0 to c41d5b4 Compare February 17, 2025 10:35
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.

1 participant