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] issue with eta.js #458

Closed floriannari closed 1 week ago

floriannari commented 4 months ago

Describe the bug

@ladjs/consolidate configures eta.js to expect a path relative to "."

new (require('eta').Eta)({
  views: '.'
})

https://github.com/ladjs/consolidate/blob/master/lib/consolidate.js#L763

So when email-templates passes it an absolute path, eta executes path.join('.', '/absolute/path/html.eta'); which removes the first "/". https://github.com/eta-dev/eta/blob/main/src/file-handling.ts#L72

So, the requested file is not found EtaError [Eta Error]: Could not find template: absolute/path/html.eta at Eta.readFile (node_modules/eta/dist/eta.umd.js:593:15) at Eta.handleCache (node_modules/eta/dist/eta.umd.js:457:37) at Eta.render (node_modules/eta/dist/eta.umd.js:483:32) at node_modules/@ladjs/consolidate/lib/consolidate.js:765:23 at node_modules/@ladjs/consolidate/lib/consolidate.js:169:5 at new Promise () at promisify (node_modules/@ladjs/consolidate/lib/consolidate.js:158:10) at exports.eta (node_modules/@ladjs/consolidate/lib/consolidate.js:758:10) at node:internal/util:375:7 at new Promise ()

By replacing

new (require('eta').Eta)({
  views: '.'
})

by

new (require('eta').Eta)({
  views: '/'
})

This solves our problem, but it would affect people using @ladjs/consolidate with relative paths

Node.js version: v18.17.1

OS version: Debian 11

Description:

Actual behavior

email-templates don't work with eta.js templating engine

Expected behavior

email-templates should work with eta.js templating engine

Code to reproduce

app.mjs

import * as nodemailer from "nodemailer";
import EmailTemplates from 'email-templates';

const transporter = nodemailer.createTransport({
    host: "https://smtp.myserver.fr",
    port: 25
});

const emailTemplates = new EmailTemplates({
    message: {
        from: "tutu@mail.com"
    },
    views: {
        root: "./email_templates",
        options: {
            extension: 'eta'
        }
    },
    transport: transporter
});

const mailOptions = {
    text: "toto",
    to: "toto@mail.com",
    subject: "tata",
}

emailTemplates.send({
    template: "a_template",
    message: mailOptions,
    locals: {
        name: "titi"
    },
})

email_templates/a_template/html.eta

Hi <%= it.name %> !

Checklist

floriannari commented 4 months ago

(I'm aware that this is mostly an @ladjs/consolidate bug and not an email-template bug. But they don't allow issues, and their project is maintained by forwardemail)

exoup commented 4 months ago

I'll note, this issue is not reproducible in a Windows environment. (That is, both absolute and relative paths just work)

In Linux (Ubuntu), the error does occur. It does seem to resolve if I prepend an extra . to the beginning of my path although I only have a headless environment to test in, so I can't confirm it's properly loaded in a browser.

@floriannari, can you confirm?

You may also be able to use a custom render function to pass in a custom eta instance, similar to (the docs)[https://github.com/forwardemail/email-templates/tree/master?tab=readme-ov-file#custom-rendering-eg-from-a-mongodb-database]

I'll admit that's not the best DX though.

floriannari commented 4 months ago

It does seem to resolve if I prepend an extra . to the beginning of my path

I'm not sure to understand where to add the "."?

exoup commented 4 months ago

In the views root path.

const emailTemplates = new EmailTemplates({
    message: {
        from: "tutu@mail.com"
    },
    views: {
        root: "../email_templates",
        options: {
            extension: 'eta'
        }
    },
    transport: transporter
});
floriannari commented 4 months ago

I have the impression that in this case, email-templates notices that the template doesn't exist (because it's in "./email-templates" and not "../email-templates").

And so @ladjs/consolidate (and therefore eta.js) are simply not called.

edit : github.com/forwardemail/email-templates/blob/master/index.js#L192

const exists = await this.templateExists(string);
if (!exists) return;

return this.render( // ...
titanism commented 1 month ago

PR welcome to fix this