Skip to content

Drop SendGrid email transport support#23392

Merged
rijkvanzanten merged 12 commits intodirectus:mainfrom
florian-strasser:patch-2
Sep 5, 2024
Merged

Drop SendGrid email transport support#23392
rijkvanzanten merged 12 commits intodirectus:mainfrom
florian-strasser:patch-2

Conversation

@florian-strasser
Copy link
Contributor

@florian-strasser florian-strasser commented Aug 17, 2024

Based on issue #23217 I removed the nodemailer-sendgrid else if branch

What's changed:

  • We are dropping support sendgrid

Potential Risks / Drawbacks

  • It's a breaking change that forces users that rely on this to switch to SMTP

Review Notes / Questions


Closes #23217

Based on issue directus#23217 I removed the nodemailer-sendgrid else if branch
@changeset-bot
Copy link

changeset-bot bot commented Aug 17, 2024

🦋 Changeset detected

Latest commit: 2c712db

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@directus/api Major
directus Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@florian-strasser
Copy link
Contributor Author

florian-strasser commented Aug 17, 2024

I would also remove the nodemailer-sendgrid package from api/package.json, but I'm not sure if it is currently used in any other file than api/src/mailer.ts

Since the package received the last update 6 years ago we remove nodemailer-sendgrid for security reasons
@florian-strasser
Copy link
Contributor Author

Not sure how to fix the current errors, probably I should not just remove the line from package.json.

 WARN  There are cyclic workspace dependencies: /home/runner/work/directus/directus/api, /home/runner/work/directus/directus/directus

ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with api/package.json

@paescuj
Copy link
Contributor

paescuj commented Aug 18, 2024

Not sure how to fix the current errors, probably I should not just remove the line from package.json.

 WARN  There are cyclic workspace dependencies: /home/runner/work/directus/directus/api, /home/runner/work/directus/directus/directus

ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with api/package.json

Heya @McSundae 👋 You need to run pnpm install to update the lockfile after the change to the package.json file.

Copy link
Contributor

@paescuj paescuj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paescuj paescuj changed the title Drop nodemailer-sendgrid support Drop SendGrid email transport support Aug 18, 2024
@paescuj paescuj marked this pull request as draft August 18, 2024 17:39
@rijkvanzanten rijkvanzanten marked this pull request as ready for review September 2, 2024 23:41
@rijkvanzanten rijkvanzanten merged commit 341d2a8 into directus:main Sep 5, 2024
@github-actions github-actions bot added this to the Next Release milestone Sep 5, 2024
@u12206050
Copy link
Contributor

WHY?!? More importantly why not release this on a minor release, it is a big breaking change to anyone using Sendgrid.

@florian-strasser
Copy link
Contributor Author

florian-strasser commented Oct 14, 2024

@u12206050 I understand your frustration, but as mentioned in the initial issue the package that was used for this is not maintained since over 6 years, so it is a security issue to keep it inside of directus. We recommend to use smtp to connect with send grid.

The package I'm talking about is:
https://github.com/nodemailer/nodemailer-sendgrid

@u12206050
Copy link
Contributor

@u12206050 I understand your frustration, but as mentioned in the initial issue the package that was used for this is not maintained since over 6 years, so it is a security issue to keep it inside of directus. We recommend to use smtp to connect with send grid.

The package I'm talking about is: https://github.com/nodemailer/nodemailer-sendgrid

Thanks, that I understand, although I still don't understand how this could be released as a patch release, are we too assume from now on that patch releases could also contain breaking changes?

@jogi-k
Copy link

jogi-k commented Oct 21, 2024

Same opinion. Why are you bringing out breaking changes in hotfixes?
Thats completely incompatible with semantic versioning

Not that I would be affected, but it directly caught my attention when looking at the release-notes.

@inovosel
Copy link

Have you added any guide/instructions how to get SendGrid working again after you have removed support for SendGrid transport? Or a way how to manually add SendGrid transport support (with me taking all the risks for using outdated component)?

@florian-strasser
Copy link
Contributor Author

As already mentioned it is recommended to use simply smtp. So you simply go into your config file and change the specific parameters:

https://docs.directus.io/self-hosted/config-options.html#smtp-smtp

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider dropping the dependency on nodemailer-sendgrid

6 participants