Skip to content
This repository has been archived by the owner on Oct 21, 2022. It is now read-only.

Move email template into ts file #19

Merged
merged 2 commits into from
Apr 6, 2020
Merged

Conversation

nfasulo
Copy link
Contributor

@nfasulo nfasulo commented Apr 2, 2020

TypeScript cannot be configured to copy arbitrary files into its output folder (microsoft/TypeScript#30835 (comment)), so the handlebars template for the welcome email is not being added to the dist folder. This leads to the following error in the compiled app:

UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory, open '/home/node/dist/lib/emails/welcomeEmail.html.hbs'

In the future we can look into using another tool to give us a more flexible pipeline than TypeScript does, but for now the fastest solution is to move this template out of its own file and into the GovDeliveryClient.ts

I have left the directory lib/emails intact, but at this point neither of the templates it contains are being used by this app.

@rtravitz
Copy link
Contributor

rtravitz commented Apr 2, 2020

If I'm understanding the problem correctly, could we have Docker copy the arbitrary file for us, and would that even be preferable?

@kalilsn
Copy link
Contributor

kalilsn commented Apr 3, 2020

@rtravitz that's an interesting idea. I think it could work but we would also probably want to mount it in dist for local development. Probably not a huge issue at the moment but could get unwieldy if we end up with more static files in the future

Copy link
Contributor

@kalilsn kalilsn left a comment

Choose a reason for hiding this comment

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

@nfasulo if we go this route, could you also delete that lib/emails directory? 🚮

Copy link
Contributor

@kalilsn kalilsn left a comment

Choose a reason for hiding this comment

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

@nfasulo if we go this route, could you also delete that lib/emails directory? 🚮

Copy link
Contributor

@rtravitz rtravitz left a comment

Choose a reason for hiding this comment

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

This is now in the hot path for having this running after Dynamo got fixed. And the template string is dead simple to understand and change.

@nfasulo nfasulo merged commit b5c2d25 into master Apr 6, 2020
@nfasulo nfasulo deleted the nf/move-email-template-inline branch April 6, 2020 14:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants