AttachmentManager Overload Definitions

[Solonotix]

🤔 What's the problem you've observed?

Ambiguous return type from the AttachmentManager, when calling this.attach() within a Cucumber step. The return type is listed as void | Promise<void>, which begs the question of whether the call must be preceded by an await. Of course, this doesn't cost anything specifically since Cucumber steps support async/await, but it takes looking at the source code to know when it must be awaited or not.

✨ Do you have a proposal for making it better?

The existing source code looks like this:

export type ICreateAttachment = (
  data: Buffer | Readable | string,
  mediaType?: string,
  callback?: () => void
) => void | Promise<void>;
// ...
export default class AttachmentManager {
  // ...
  create(
    data: Buffer | Readable | string,
    mediaType?: string,
    callback?: () => void
  ): void | Promise<void> {
    // ...
  }
  // ...
}

The ambiguity could be eliminated by having an overload signature as follows:

export type ICreateBufferAttachment = (
  data: Buffer,
  mediaType: string
) => void;

export type ICreateStringAttachment = (
  data: string,
  mediaType?: string
) => void;

export type ICreateStreamAttachment = {
  data: Readable,
  mediaType: string,
  callback: () => void
) => Promise<void>;

export type ICreateAttachment = ICreateBufferAttachment | ICreateStringAttachment | ICreateStreamAttachment;

export default class AttachmentManager {
  // ...
  create(data: string, mediaType?: string): void;
  create(data: Buffer, mediaType: string): void;
  create(data: Readable, mediaType: string, callback: () => void): Promise<void>;
  create(data: Buffer | Readable | string, mediaType?: string, callback?: () => void): void | Promise<void> {
    // ...
  }
  // ...
}

It would also be helpful to have a type defined for the supported values of media type. At first, it looks like MIME types because of values like mediaType = 'image/png', but then there are your custom media types such as log messages as mediaType = 'text/x.cucumber.log+plain'. I checked the @cucumber/messages repo as well, but saw that it's also defined as just string for the type.

📚 Any additional context?

No additional context. I'm open to suggestions on the above. I'm not an expert in TypeScript, by any means, but the ambiguity has driven me nuts for a while, as I was never quite sure (until just now) whether the attachment needed to be awaited or not.

Edits

1. Just realized the callback function is only a viable option on the Readable code path, and all others ignore the argument entirely. Further, callback is a required argument in that flow, so I altered the type definition to reflect this.