nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
24.1k stars 3.34k forks source link

EmailProvider: make nodemailer an optional dependency if one uses external services like MailJet #3012

Closed tpinne closed 2 years ago

tpinne commented 2 years ago

Description 📓

This is definitely in the category "nice to have".

It would be great if the default implementation somehow could make the dependency to nodemailer optional if someone provides a fully custom sendVerificationRequest function.

I for example use MailJet to send out those transactional emails. It works flawlessly, there is even nothing more to configure than sendVerificationRequest function itself. But to be able to run the code I must install nodemailer to get the provider to work at all.

This isn't a big deal. It just wouldn't be necessary and would even reduce the bundle size. But then again, this is only server side. But who doesn't love a smaller bundle anywhere :)

How to reproduce ☕️

There is nothing to reproduce.

Contributing 🙌🏽

No, I am afraid I cannot help regarding this

balazsorban44 commented 2 years ago

It is going to be optional in v4. 😊 Exactly for the reason you are describing, to reduce bundle size for those who don't need it.

https://next-auth.js.org/getting-started/upgrade-v4#nodemailer

tpinne commented 2 years ago

Ehm, but I'm on 4.0.0-beta.4 and had to install nodemailer. Otherwise I get an error immediately when starting up the project.

yarn run v1.22.5
$ next dev
ready - started server on 0.0.0.0:3000, url: http://localhost:3000
info  - Loaded env from /Users/tpp/_projects/bcup-members/.env
info  - Using webpack 5. Reason: Enabled by default https://nextjs.org/docs/messages/webpack5

   ┌─────────────────────────────────────────────────┐
   │                                                 │
   │   ◈ Server now ready on http://localhost:8888   │
   │                                                 │
   └─────────────────────────────────────────────────┘

event - compiled successfully
event - build page: /api/member-invites/[id]
wait  - compiling...
event - build page: /next/dist/pages/_error
event - build page: /api/member/current
error while proxying request: socket hang up
error while proxying request: socket hang up
error while proxying request: socket hang up
event - build page: /api/auth/[...nextauth]
event - build page: /api/auth/[...nextauth]
event - compiled successfully
wait  - compiling...
Error: Cannot find module 'nodemailer'
Require stack:
- /Users/tpp/_projects/bcup-members/node_modules/next-auth/providers/email.js
- /Users/tpp/_projects/bcup-members/.next/server/pages/api/auth/[...nextauth].js
- /Users/tpp/_projects/bcup-members/node_modules/next/dist/next-server/server/next-server.js
- /Users/tpp/_projects/bcup-members/node_modules/next/dist/server/next.js
- /Users/tpp/_projects/bcup-members/node_modules/next/dist/server/lib/start-server.js
- /Users/tpp/_projects/bcup-members/node_modules/next/dist/cli/next-dev.js
- /Users/tpp/_projects/bcup-members/node_modules/next/dist/bin/next
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.mod._resolveFilename (/Users/tpp/_projects/bcup-members/node_modules/next/dist/build/webpack/require-hook.js:4:1855)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/Users/tpp/_projects/bcup-members/node_modules/next-auth/providers/email.js:8:19)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/tpp/_projects/bcup-members/node_modules/next-auth/providers/email.js',
    '/Users/tpp/_projects/bcup-members/.next/server/pages/api/auth/[...nextauth].js',
    '/Users/tpp/_projects/bcup-members/node_modules/next/dist/next-server/server/next-server.js',
    '/Users/tpp/_projects/bcup-members/node_modules/next/dist/server/next.js',
    '/Users/tpp/_projects/bcup-members/node_modules/next/dist/server/lib/start-server.js',
    '/Users/tpp/_projects/bcup-members/node_modules/next/dist/cli/next-dev.js',
    '/Users/tpp/_projects/bcup-members/node_modules/next/dist/bin/next'
  ]
}
tpinne commented 2 years ago

There is a notable difference between the next2 release and the current beta4. Looks like a regression.

Here it is dynamically imported: https://github.com/nextauthjs/next-auth/blob/v4.0.0-next.2/src/providers/email.js

Here not anymore: https://github.com/nextauthjs/next-auth/blob/v4.0.0-beta.4/src/providers/email.ts

balazsorban44 commented 2 years ago

You should be creating your custom email provider, nodemailer is still part of the built-in one, it's just not bundled together with everything else.

Here is the source code to give you an idea: https://github.com/nextauthjs/next-auth/blob/beta/src/providers/email.ts

tpinne commented 2 years ago

It's directly imported right in the first line. Or am I missing something?

tpinne commented 2 years ago

Ah now I get it. Create a complete custom provider....

Although I liked the approach in https://github.com/nextauthjs/next-auth/blob/v4.0.0-next.2/src/providers/email.js to require it inside the sendVerificationRequest function.

tpinne commented 2 years ago

It is going to be optional in v4. 😊 Exactly for the reason you are describing, to reduce bundle size for those who don't need it.

https://next-auth.js.org/getting-started/upgrade-v4#nodemailer

Ok, last comment on this... You maybe should update the docs then. Because they refer to the way I used it. Using the builtin provider, but with a custom sendVerificationRequest function. This works according to the upgrade guide, because of the changes introduced in next.2. But with the latest changes the docs don't reflect the reality anymore to 100%.

balazsorban44 commented 2 years ago

Conditional imports are hard, I tried before, it failed. The code you referred to didn't even compile correctly for me.

Much easier if you just create your own custom provider.

Not sure what should be updated in the docs. The built-in provider still uses nodemailer, and probably will, unless we find a better alternative in the future.

tpinne commented 2 years ago

Currently the docs state:

If you are using the Email provider you must install it in your project manually, or use any other Email library in the sendVerificationRequest callback.

This suggests that you either install nodemailer OR use a custom sendVerificationRequest callback. While that was true with the changes made in next.2 (because the nodemailer import was made dynamic inside the callback), it's not anymore with the latest beta, where the import is back to mandatory but the whole provider is optional.

So you have to create an own provider to take advantage of reduzed bundle size together with a custom sendVerificationRequest callback like you pointed out in a comment earlier here.

But that is not obvious from the current state of the docs.

I hope I got my point across :-)

balazsorban44 commented 2 years ago

I won't be making the nodemailer import dynamic again, so what's your suggestion? the built-in email provider also exposes config options specific to nodemailer.

tpinne commented 2 years ago

I don't want you to change anything code wise. I only wanted to point out, that imho the current docs do not reflect the current implementation and suggest to update it to the current situation.

With the docs for next.2 the following has been possible without adding nodemailer as dependency:

const options: NextAuthOptions = {
  // ...
  providers: [
    EmailProvider({
      sendVerificationRequest: sendMagicSignInLink,
    })
  ],
};

This commit https://github.com/nextauthjs/next-auth/commit/e099223a27caf2b97275e36a868a0882a7ef121c changed that and might lead to misunderstandings.

With that change the above solution (which would have worked before that commit, according to the docs) only works with installing nodemailer. Which is obvious if you look at the original source.

But then you clarified that I need to provide a custom provider for my purpose to use any other Email library.

Now I have the following config which works without installing nodemailer. Because of TS typings it forced me to provide a server and options property, but that doesn't bother me.

const options: NextAuthOptions = {
  // ...
  providers: [
    {
      id: 'email',
      type: 'email',
      name: 'E-Mail',
      server: '',
      options: {},
      sendVerificationRequest: sendMagicSignInLink,
    },
  ],
};

But I would never have come to that conclusion based on the current (upgrade) docs.

Zhumatic commented 9 months ago

@tpinne just tried your method now doesn't works anymore. But this is a new working method to use email verification without installing the nodemailer package.

  1. created a customized email sending function, in my case i just call it sendVerificationRequest, saved it in lib folder of my app.

  2. pass the custom email sending funtion to nextauth config

    
    import { sendVerificationRequest } from "./verification";

// adding the custom provider into the config // i am using aws ses so the id is set as "ses" export const config = { adapter: PrismaAdapter(prisma), providers: [ { id: "ses", type: "email", sendVerificationRequest, }, GoogleProvider({ clientId: process.env.GOOGLE_CLIENT_ID, clientSecret: process.env.GOOGLE_CLIENT_SECRET, }), ],


3. when call signin function, use "ses" instead of the default "email"
```js
signIn("ses", {
        email: data.email,
        callbackUrl
      });

nodemailer is currently producing [The punycode module is deprecated] (https://stackoverflow.com/questions/77443850/nodemailer-deprecationwarning-the-punycode-module-is-deprecated) warnings, and the nodemailer author said he will change anything until node 22. it's quite annoying seeing the error being logged.