Sendgrid-nodejs: Attachment helper for base64 encoding

Created on 17 Oct 2017  路  8Comments  路  Source: sendgrid/sendgrid-nodejs

We have an example in USE_CASES.md on how to handle attachments.

We need to update the Attachment object such that it can also take in a raw file, rather than a base64 encoded file, and perform the base64 encoding behind the scenes.

Note that the total size of your email, including attachments, must be less than 30MB. The total size is calculated from the entire JSON payload. Note that you will need to handle the case where UTF-8 characters are included.

medium work in progress sendgrid enhancement

Most helpful comment

I think that the simpler approach of reading the whole file makes sense here, because it's a helper, and for 98% of the use cases where someone just wants to include a file, it will work. With the email size limit being 30mb anyway, it's unlikely one will try to read files larger than that which would not fit in memory.

For cases where it causes problems, developers could roll their own solution and provide a buffer instead of a file.

With the configuration option and two ways of doing it I think you may run the risk of over complicating things for no real good reason, and it's possible that the chunked way of reading may still not suite everyone's needs.

That's why I think it's best to provide a simple solution that just works in most cases, while giving developers the option to build something themselves. Accepting buffers already does the latter.

Also make sure to use ES6 code style (e.g. using const, let where appropriate instead of var).

All 8 comments

Hi @thinkingserious, I'm working in this one right now.

See this gist, based on it, which approach do you think suits better here sample1 (read the entire file) or sample2 (read chunks)?

I'm doing this to be synchronous, is it ok?

Hi @spelcaster,

Thanks for the PR, I've added to my backlog for review. My initial thoughts is that it may be useful to provide configurable options to the user with a good default.

That said, @adamreisnz may weigh in before I get a chance to.

With Best Regards,

Elmer

I think that the simpler approach of reading the whole file makes sense here, because it's a helper, and for 98% of the use cases where someone just wants to include a file, it will work. With the email size limit being 30mb anyway, it's unlikely one will try to read files larger than that which would not fit in memory.

For cases where it causes problems, developers could roll their own solution and provide a buffer instead of a file.

With the configuration option and two ways of doing it I think you may run the risk of over complicating things for no real good reason, and it's possible that the chunked way of reading may still not suite everyone's needs.

That's why I think it's best to provide a simple solution that just works in most cases, while giving developers the option to build something themselves. Accepting buffers already does the latter.

Also make sure to use ES6 code style (e.g. using const, let where appropriate instead of var).

I'll stick with the sample1.

How the Attachment should know that it needs to load the file from file system? A new prop in object passed to fromData or a new branch in the Attachment constructor?

Follow #505

Why do you guys require people to base64 encode files?

Hello @LukeXF,

This is a requirement of the underlying API. The purpose is this issue is to hide that implementation detail from users of this SDK.

With Best Regards,

Elmer

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kiranshashiny picture kiranshashiny  路  4Comments

TobiahRex picture TobiahRex  路  3Comments

prasoonjalan picture prasoonjalan  路  3Comments

nicoasp picture nicoasp  路  3Comments

Chrischuck picture Chrischuck  路  3Comments