forwardemail / email-templates

Create, preview (browser/iOS Simulator), and send custom email templates for Node.js. Made for @forwardemail, @ladjs, @cabinjs, @spamscanner, and @breejs.
https://forwardemail.net/docs/send-emails-with-node-js-javascript
MIT License
3.64k stars 339 forks source link

[fix] Move preview-email to devDeps #453

Closed felixmosh closed 10 months ago

felixmosh commented 10 months ago

Describe the bug

Since preview-email is for development purposes, there is no real good reason to make it a dependency of this lib. It should be defined as devDep, and required within the condition that enables it.

titanism commented 1 week ago

@felixmosh v12.0.0 of email-templates has been release to GitHub and npm with optional dependency of preview-email. In production environments it will console.error if you attempt to have previewEmail option with a truthy value, and in production environments it will throw.

Thank you for your feedback and patience @felixmosh

Consider supporting our efforts at https://forwardemail.net 🙏

https://github.com/forwardemail/email-templates/releases/tag/v12.0.0

felixmosh commented 1 week ago

I've tested the new version, with Yarn, looks like it still installs it :|

Since this lib only passes users config.preview to the lib, I think that it's better to make config.preview the lib itself, WDYT? Before

image

After

image

12 Mb of extra bloat

titanism commented 1 week ago

You may need to use yarn install --ignore-optional or yarn workspaces focus --production. Also note that NODE_ENV=production should prevent installing devDependencies. We do not use yarn.

felixmosh commented 1 week ago

Sure, my tests were with yarn workspaces focus --production

titanism commented 1 week ago

We suggest to use pnpm or figure out the underlying reason and submit a PR. We don't suggest to pass the function, it's an anti-pattern.

You already have 661M, and you're upset about 12MB extra? I think there are other things to be concerned about with time.

felixmosh commented 1 week ago

Yeah, that is right, but this is one of the big fishes :]

titanism commented 1 week ago

Don't you need to regenerate (delete/recreate) your lockfile completely after a big change like this?

https://github.com/yarnpkg/berry/issues/2425

felixmosh commented 1 week ago

I've tried it, no luck

What is the antipattern by passing the a function / the lib instance?