feat: auto bind event exchanges to endpoints #6
Conversation
There was a problem hiding this comment.
Overall this looks good. I'd like to see some tests on the changes that you are making. I know that the test coverage is lacking but we need to start addressing that.
Additionally, if you have any links to why a library should NOT have package-lock.json I'd be interested to read them. Most of the links that I found are rather stale and can be found to argue each way.
Also , this PR has commits from another PR that post merge may cause issues. Please consider isolating commits to a single PR to simplify reviews.
| @@ -127,7 +127,7 @@ export class Transport extends EventEmitter implements Transport { | |||
| if (this.channel) { | |||
| let channel = this.channel; | |||
| return new Promise((resolve, reject) => { | |||
| const result = channel.publish(exchange, routingKey, body, {persistent: true}, | |||
| const result = channel.publish(exchange, routingKey, body, {persistent: true, contentType: "application/vnd.masstransit+json"}, | |||
There was a problem hiding this comment.
Can we get a unit test on this? I know that the unit test coverage is low for this project but any help there would be appreciated.
There was a problem hiding this comment.
I've added some tests for the publish mechanism as well as the delayed publishing when sending messages before the channel connection was established.
| @@ -135,7 +135,7 @@ class MassTransitBus extends EventEmitter implements Bus { | |||
| console.log('Connecting', this.hostAddress.toString()); | |||
|
|
|||
| try { | |||
| this.connection = connect(this.hostAddress + '?heartbeat=60'); | |||
| this.connection = connect(this.hostAddress + '?heartbeat=60') as unknown as Promise<Connection>; | |||
There was a problem hiding this comment.
how does casting it to unknown then Promise help? Why the as unknown at all?
There was a problem hiding this comment.
The bluebird promise type seems to be incompatible with the native Promise type which is circumvented by the unknown cast.
When I cast directly to Promise I receive the following error:
TS2352: Conversion of type 'Bluebird' to type 'Promise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first. The types returned by 'then(...)' are incompatible between these types. Property '[Symbol.toStringTag]' is missing in type 'Bluebird' but required in type 'Promise'.
If you have a better idea I'll gladly change it.
|
|
||
| toExchange(): string { | ||
| return `${this.ns}:${this.name}` | ||
| } |
There was a problem hiding this comment.
This we can get a test on pretty easily
| for (const messageType of this.boundEvents) { | ||
| await channel.bindExchange(this.queueName, messageType.toExchange(), ''); | ||
| } | ||
|
|
There was a problem hiding this comment.
Added tests for the configureTopology method.
|
Alright, I'll remove the remove precompiled commit as it is part of #5 anyway. |
maxjoehnk
force-pushed
the
feature/auto-bind-event
branch
from
33305fd to
46e57d1
Compare
|
Rebased/merged. |
closes #4