search

nodemailer / mailparser

Decode mime formatted e-mails
Other
1.59k stars 281 forks source link

"html": false #309

Open joelterry opened 2 years ago

joelterry commented 2 years ago

I have seen a parsed email object with the html field set to false, and have noticed a handful of issues that mention this. Here are a few examples:

https://github.com/nodemailer/mailparser/issues/185 https://github.com/nodemailer/mailparser/issues/187 https://github.com/nodemailer/mailparser/issues/252

My issue isn't even about whether the parser correctly or incorrectly omitted the HTML, but rather the use of false as an empty/uninitialized string value, which the code seems to be doing: https://github.com/nodemailer/mailparser/blob/master/lib/mail-parser.js#L152.

Is there any reason why you're using false instead of null, undefined, or ""? People who see these false values will likely misunderstand the email object schema (as in .html indicates the presence of HTML rather than holds its content). Additionally, if this object is sent further down the wire, people will choose to encode/decode with the assumption that .html is a string, or maybe an optional string, but probably not a string|boolean.

That being said, that's just my gut feeling, let me know if I'm missing something. I also understand if this project has too much momentum to make such a change now.

Ionys320 commented 1 year ago

I agree that using null or even undefined would probably be better, allowing for example to do mail.html ?? mail.textAsHtml (may already work in JavaScript, but not in TypeScript because of the type of the html property) instead of mail.html != false ? mail.html : mail.textAsHtml.