Expose types

[JakeSidSmith]

Overview

Expose all types so these can be used by the consumer.

Use case

I want to wrap my Mailgun calls in another function that's functionality differs per environment. In the below case I cannot import MailgunMessageData or MessageSendResult that are used by the create method.

export const sendEmail = async (data: MailgunMessageData): Promise<MessageSendResult> => {
  if (process.env.NODE_ENV === 'production') {
    const result = await client.messages.create('domain.com', data);

    return result;
  }

  console.log(`Sending email: ${JSON.stringify(data)}`);
  
  return { /* something equivalent to MessageSendResult */ };
};

Changes

As part of this change this PR also:

• Renames the interfaces directory to types as it also contains type aliases and enum definitions
• Renames the following conflicting types
  • types/list.ts ValidationResponse -> ListValidationResponse
  • types/list.ts ValidationResult -> ListValidationResult
  • types/Webhooks.ts ValidationResponse -> WebhooksValidationResponse

I don't believe any of these changes to be breaking changes as none of the changed types/paths were previously exposed and should be considered internal.

[olexandr-mazepa]

Hello, @JakeSidSmith thank you for working on this and excuse me for writing a comment before you asked for the review, I just want to save some of your time.
It seems I don't fully understand the issue that this PR will fix. As I know that MailgunMessageData and MessageSendResult types can be imported in the following way:

import { MailgunMessageData, MessagesSendResult } from 'mailgun.js/interfaces/Messages';
...
const sendEmail = async (messageBody: MailgunMessageData): Promise<MessagesSendResult> => {
  const res: MessagesSendResult = await mg.messages.create(domain, messageBody)
  console.log(res);
  return res;
};

Also renaming the interfaces folder to the types is a breaking change because people may already have imports from this folder.
I agree that there is some inconsistency in the fact that interfaces files also include types and enums. I think It would be better to create a few new folders and move those definitions to the new places than rename the original folder. Just because it will decrease the number of changes in client code regarding imports instructions.

[JakeSidSmith]

Hey, @olexandr-mazepa, thanks for the quick response. 😁

I understand what you're saying about being able to import from /interfaces, and although I'd considered using this approach, I generally avoid it (unless explicitly documented) because I'd assume those directories were internals. I can't see anywhere in the docs for mailgun.js that such imports are referenced.

If you were to document it, you then run into the above issue you've highlighted, where you cannot refactor the structure of your library because users are already using that directory structure.

It's also not obvious which, if any, of those directories/modules I can/should import from. Presumably some of the other files should not be imported from directly as you want to conceal the implementation and only expose the API you want your users to utilize.

Anyway, those are a few of my reasoning behind the change, but the main one is accessibility. Using an editor like VSCode, with intellisense, I've become accustomed to writing out the name of a thing I want to use, and expect my editor to add the import for me, but the types within interfaces cannot be automatically imported as it stands.

I'd be happy to change the directory name back to interfaces if you prefer, but I would like some way to access the types at the root of the library, and it'd probably save you some hassle when it comes to refactoring, as mentioned above, if you advise your consumers to only import things from the root.

Exporting them all as is I have in this PR may not be the ideal approach. Perhaps you'd prefer them namespaced? Happy to figure that out together. 😊

Side note: importing from lots of sub-modules also crowds your imports a bit, and doesn't allow people to easily namespace your types e.g. Mailgun.MessageData which can be a problem when the consuming project has conflicting type names.

[olexandr-mazepa]

Okay, I got the idea and it makes sense to me.
Thank you for the additional details.
Let's continue work on this but I see a few things that need to be addressed before merging:

• Based on the history of the issues I know that people already use the current way of importing, so let's mark this update as a breaking change.
• Let's don't rename the interfaces folder for now because the bigger part of the exported items in files are interfaces. I assume it doesn't matter where files are physically placed in the way of exporting that you suggest, so let's leave the old name and decrease the number of changes.
  I will do the movement of enums and types in the next iterations.

And the last one I like the idea of using namespaces and hiding some interfaces that aren't useful for client code like descriptions for API responses but I don't want to make your hard time working on current changes.
In a few words, If you think you have time and motivation to do this you are welcome, if not it is also okay.
Thank you for your contribution one more time.

[JakeSidSmith]

@olexandr-mazepa I'm happy to help out, even in non-billable hours. 😊

If there are some types that you don't want exposed to the world (those used internally by functions for example) you can use this compiler option coupled with doc comments to prevent the types being exported, which would also reduce the size of your published package.

I won't have a chance to look a this until this weekend probably, but will see what I can come up with then, that would maintain some backward compatibility.

[JakeSidSmith]

Still been super busy, but thought I'd share what I'm working with for the moment.

To avoiding using the internals I'm doing the following to alias the types I needed:

const client = mailgun.client({/* etc */});

type MailgunMessageData = Parameters<typeof client.messages.create>[1];

type MessagesSendResult = Awaited<ReturnType<typeof client.messages.create>>;

[samuliasmala]

It's also possible to define the types without a client instance if needed:

import Mailgun from 'mailgun.js';

type MailgunClient = ReturnType<Mailgun['client']>;
type MailgunMessageData = Parameters<MailgunClient['messages']['create']>[1];