added SMTP support and removed resend dependency#313
added SMTP support and removed resend dependency#313Chigala wants to merge 5 commits intotriggerdotdev:mainfrom
Conversation
|
There was a problem hiding this comment.
As well as these changes, we will need to update the following docs
- Contributing guide
- The docker repo: https://github.com/triggerdotdev/docker
- The .env.example
- The fly.io repo: https://github.com/triggerdotdev/fly.io
- The fly.io self hosting guide: https://trigger.dev/docs/documentation/guides/self-hosting/flyio
- The Render self hosting guide: https://trigger.dev/docs/documentation/guides/self-hosting/render
|
You may notice some issues with SMTP depending on provider. Might be worth enabling pooling if that happens (yes, that's my code word now). |
|
Lol, I'll definitely look into this. Thanks |
matt-aitken
left a comment
matt-aitken
left a comment
There was a problem hiding this comment.
This works! I had to make a small change to coerce the env var string to a number
|
@Chigala to merge this that checklist above from @ericallam needs to be completed |
|
@matt-aitken I've already sent pull requests to the docker and fiy.io repositories. |
packages/emails/src/index.tsx
Outdated
| from: string; | ||
| replyTo: string; | ||
| }) { | ||
| this.#client = config.smtpConfig ? nodemailer.createTransport(config.smtpConfig) : undefined; |
There was a problem hiding this comment.
this smtpConfig option is always going to be "truthy" so the nodemailer.createTransport thing is always going to be called:
There was a problem hiding this comment.
I'm sorry but I don't quite understand this
There was a problem hiding this comment.
@ericallam Should I put back the resend api key and dependency as a fallback to when the smtp config doesn't exist?
There was a problem hiding this comment.
I think Eric just meant the config values should be checked instead of the entire object. Otherwise, this will never return undefined. Checking for truthy smtpConfig.host should probably be enough.
There was a problem hiding this comment.
Ohh, makes sense. Thanks
|
@Chigala when you get a chance could you check my comment above? |
|
is this planned? I can recreate the PR and update the code if @Chigala is not available |
|
also happy to help work on a revival of this 🙏 |
6 participants
Description
Added SMTP support
Changes
fixes #282