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(amqp/connection.ts): add publishOptions to RequestOptions; use them in AmqpConnection.request #723

Merged
merged 1 commit into from
Apr 15, 2024
Merged

feat(amqp/connection.ts): add publishOptions to RequestOptions; use them in AmqpConnection.request #723

merged 1 commit into from
Apr 15, 2024

Conversation

davidmwhynot
Copy link
Contributor

Added a new property named "publishOptions" to the RequestOptions interface so that the additional
options available in the amqplib's Options.Publish interface can be provided when making rpc
requests. Added said usage of the new property to the AmqpConnection.request method's call to
this.publish.

re #719

…hem in AmqpConnection.request

Added a new property named "publishOptions" to the RequestOptions interface so that the additional
options available in the amqplib's Options.Publish interface can be provided when making rpc
requests. Added said usage of the new property to the AmqpConnection.request method's call to
this.publish.

re #719
@davidmwhynot
Copy link
Contributor Author

davidmwhynot commented Apr 15, 2024

This could also have been done like so:

export interface RequestOptions extends Omit<Options.Publish, 'replyTo'> {
  exchange: string;
  routingKey: string;
  timeout?: number;
  payload?: any;
}

// usage in AmqpConnection.request:
await this.publish(
  requestOptions.exchange,
  requestOptions.routingKey,
  payload,
  {
    ...requestOptions,
    replyTo: DIRECT_REPLY_QUEUE,
    correlationId,
  }
);

however, I did not want to pollute the existing RequestOptions interface with the relatively large number of additional options on the amqplib's Options.Publish interface.

I'm will to adjust to this style if it's preferred over the current change.

Copy link
Contributor

@underfisk underfisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this change. It does feel better to me as we don't need to keep mapping 1:1 each time a user needs to override something

@underfisk underfisk enabled auto-merge (squash) April 15, 2024 20:48
@underfisk underfisk merged commit 3a8d5b3 into golevelup:master Apr 15, 2024
3 checks passed
WonderPanda pushed a commit that referenced this pull request Apr 15, 2024
…nection.request (#723)

Added a new property named "publishOptions" to the RequestOptions interface so that the additional
options available in the amqplib's Options.Publish interface can be provided when making rpc
requests. Added said usage of the new property to the AmqpConnection.request method's call to
this.publish.

re #719
@WonderPanda
Copy link
Collaborator

Published as @golevelup/nestjs-rabbitmq@5.3.0

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.

3 participants