nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
67.75k stars 7.63k forks source link

Add official @nestjs/mail module #5388

Closed twister21 closed 3 years ago

twister21 commented 4 years ago

Feature Request

Nest.js should have a mail module that provides an integration for nodemailer/email-templates. There are already third-party solutions available, for example:

However, I suggest to provide an official solution like other major web frameworks (Laravel, Ruby on Rails or Spring Boot), as most business cases require mail services (e.g. password reset, notifications).

Describe the solution you'd like

@Module({
  imports: [
    MailModule.forRootAsync({
      useFactory: () => ({
        transport: 'smtps://user@example.com:password@mail.example.com',
        defaults: {
          from: 'Example <noreply@example.com>',
        },
        template: {
          dir: __dirname + '/templates',
          ...
        },
      }),
    }),
  ],
})
export class AppModule {}
jmcdo29 commented 4 years ago

So, if Nest were to create an "official" module, what package would be the underlying one? @nestjs/bull uses bull under the hood. @nestjs/typeorm uses typeorm. So what would @nestjs/mailer use?

Personally, I don't see the need for Nest as an organization to create another wrapper module around a package when the community has already done so.

twister21 commented 4 years ago

@nest-modules/mailer is based on nodemailer, but the template system was developed by the authors themselves. Unfortunately, EJS templates are broken at the moment and it supports neither multipart emails (HTML and text) nor multilingual templates.

email-templates is used by lad (another Node.js framework) and supports not only multipart emails and all common template engines but also i18n (based on @ladjs/i18n).

So, the Nest.js module could just be a wrapper for a fully-fledged package like email-templates or implement the template system itself (maybe with a nest-specific i18n module).

smolinari commented 4 years ago

To me, templating emails and sending emails are two different domains, so they should theoretically be two different modules.

Scott

twister21 commented 4 years ago

I don't think there is a need to have separate modules, as in most environments it's best practice to create template files for the different use cases (e.g. email confirmation, password reset, daily activity report).

Nevertheless, it should be possible to pass just a raw HTML string. This is how it's done in the libraries I referred to in my first comment.

smolinari commented 4 years ago

Yes, but some people might want to do their email templating differently. So, what I am getting at is, the email module from Nest should be only for email transfer. Then the community can come up with modules for templating them.

Scott

twister21 commented 4 years ago

But why should Nest not provide a solution out-of-the-box? I don't think that the majority of Nest.js users need to implement their own email templating aside from EJS, Handlebars, Pug (and many more).

In my opinion, email-templates therefore meets all requirements for a solid mail package that @nestjs/mail could rely on.

jmcdo29 commented 4 years ago

@jonasbeyer I was able to find this comment again, which basically reiterates the point I first made: https://github.com/nestjs/nest/issues/4007#issuecomment-582779510

smolinari commented 4 years ago

But why should Nest not provide a solution out-of-the-box?

Because it adds to the maintenance overhead with little advantage to the framework and it really isn't necessary that the Nest team do it all.

Scott

Dominic-Preap commented 4 years ago

I completely agree with @jmcdo29 and also have a https://github.com/nestjs/nest/issues/5148#issuecomment-664003442 as well. You can just create your own module with advantage of any customization you want, so you don't face any restriction or limitation from the framework itself. For me I like creating my own wrapper module as much as I like without worry any compatibility as long as you follow the Nest Module document.

image

chrisdevereux commented 3 years ago

The advantage of NestJS providing this is that we get some convention over configuration and it becomes easier to share higher-level third-party or in-house libraries around stuff like user onboarding (or anything else that needs to send email). However, I understand completely the maintainers wanting to 'do fewer things well' and not bloat the framework.

Maybe a compromise would be to provide a standardised interface and injection token for a mailer service as an official module? Remaining agnostic about what implementation is used, but keeping the option of providing a default implementation in the future?

underfisk commented 3 years ago

@jacob87o2 Keep in mind that the mailer module would have to provide an interface facade in other to extend or provide custom transports, right now most of the mailer libraries assume you use SMTP but what if you rely on external packages or mailer services rest API? We need to have that in mind, therefore i think nestjs should avoid having external packages for this but create its own abstraction like CQRS with the basic stuff necessary

j3bb9z commented 3 years ago

@jacob87o2

@underfisk I don't recall taking part in this conversation. 😉

underfisk commented 3 years ago

@jacob87o2

@underfisk I don't recall taking part in this conversation. 😉

Oh sorry, i meant @jonasbeyer

hood commented 3 years ago

@underfisk At this point a @nestjs/smtp module would make more sense, if any. Other kinds of integrations (REST APIs, etc) would be too vendor specific to be incorporated, IMHO.

underfisk commented 3 years ago

@hood i agree, something not so agnostic

kamilmysliwiec commented 3 years ago

We provide integrations with some of the external packages to make it easier for developers to use them in their projects. For example, @nestjs/typeorm provides an automated way to register all referenced entities in the main connection, automatically registers repositories as providers, and exposes a set of decorators to interact with them + maintains the connection, and automatically closes it (as a reaction to application lifecycle hooks). Likewise, @nestjs/bull makes it easier to register queues through Nest modules and register job processors using decorators (allowing them to be "per-job" scoped too). Similarly, @nestjs/mongoose registers a connection, provides a simple interface to define models from schemas, and exports factories & decorators to create schemas in a more declarative-way.

So as you can see, the primary goal of all these packages is to make the integration process a lot easier. With packages like nodemailer or email-templates (or even unrelated to this thread packages for hashing like bcrypt or argon2, etc etc), there are no visible benefits that such integration would bring. As @Dominic-Preap mentioned above https://github.com/Dominic-Preap (and I've mentioned here https://github.com/nestjs/nest/issues/4007#issuecomment-582779510), Nest was designed in a way that lets you integrate whatever you want by creating your own modules. For example, creating a module that would wrap the nodemailer package shouldn't really take longer than a few minutes (after reading the "Overview" category of NestJS docs).

TL;DR; You can use any existing Node.js mailer package with Nest (either nodemailer or email-templates or anything else) and you don't need the official wrapper for this. In fact, having the official wrapper module for such packages would make them less customizable and flexible. Also, as @smolinari mentioned https://github.com/nestjs/nest/issues/5388#issuecomment-692806277, having the official wrapper would add to the maintenance overhead with minimal (or 0) advantage to the framework.