Implement AMQP publisherManager #144
Conversation
| 'lib/**/*.test.ts', | ||
| 'test/**/*.*', | ||
| 'lib/types/**/*.*', | ||
| 'lib/AmqpExchangePublisherManager.ts', |
There was a problem hiding this comment.
it's problematic to write tests for this functionality, as we don't yet have any logic in place for autocreating exchanges and autosubscribing queues. this will be done next.
| publishOptions: {}, | ||
| } | ||
|
|
||
| export class AbstractAmqpQueuePublisher< |
There was a problem hiding this comment.
This is a replacement for previously existing AbstractAmqpPublisher.
It's a breaking change.
| publishOptions: Options.Publish | ||
| } | ||
|
|
||
| export class AbstractAmqpExchangePublisher< |
There was a problem hiding this comment.
This exposes RabbitMQ PubSub functionality (SNS-like).
previously we only had direct publishing to queues (SQS-like)
| ) { | ||
| super(dependencies, { | ||
| ...options, | ||
| // FixMe exchange publisher doesn't need queue at all |
There was a problem hiding this comment.
will be addressed during auto-init rework, as we need to implement exchange and subscription support
| z.infer<T['schema']>, | ||
| 'type' | 'payload' | ||
| > & { id?: string } | ||
| export type MessagePublishType<T extends CommonEventDefinition> = z.infer<T['schema']> |
There was a problem hiding this comment.
Unfortunately, previous Pick-based implementation was breaking payload type inference during publishing. TS started accepting any combination of valid message types and payloads, and not enforcing that they belong to the same message
| @@ -50,7 +43,13 @@ describe('SnsPublisherManager', () => { | |||
| await fakeConsumer.start() | |||
|
|
|||
| // When | |||
| const publishedMessage = await publisherManager.publish(TestEvents.created.snsTopic, message) | |||
| const publishedMessage = await publisherManager.publish(TestEvents.created.snsTopic, { | |||
| ...publisherManager.resolveBaseFields(), | |||
There was a problem hiding this comment.
I wish we could figure out a better way, that would both preserve specific payload type here, and not require explicitly passing all of the core fields, but so far I wasn't very successful in this.
| publisherManager.publish(TestEvents.created.snsTopic, { | ||
| ...publisherManager.resolveBaseFields(), | ||
| payload: { | ||
| // @ts-expect-error This should be causing a compilation error |
There was a problem hiding this comment.
this test can be used for checking the payload/type inference logic
| } from './AmqpQueuePublisherManager' | ||
| import { CommonAmqpExchangePublisherFactory } from './CommonAmqpPublisherFactory' | ||
|
|
||
| import Omit = util.Omit |
There was a problem hiding this comment.
🟢 nit: Importing omit could cause confusing with TS Omit, I would prefer using util.Omit or even z.util.Omit (I think it is coming from zod, sorry if I misunderstood it) to make it more specific. But this is a very nitpicky comment, feel free to ignore
There was a problem hiding this comment.
good catch, this was autoresolve by WebStorm, not intentional. I wanted to use TS Omit
| T extends AbstractAmqpPublisher<EventType, MO>, | ||
| MO, | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| PO extends Omit<AMQPPublisherOptions<any>, 'creationConfig'>, |
There was a problem hiding this comment.
Not sure if possible, but just wondering if could be possible to use unknown instead of any
There was a problem hiding this comment.
not unknown, but I'll try to wrestle with types to make this unnecessary
No description provided.