mailparser icon indicating copy to clipboard operation
mailparser copied to clipboard

"html": false

Open joelterry opened this issue 3 years ago • 1 comments

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.

joelterry avatar Jan 10 '22 02:01 joelterry

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.

Ionys320 avatar Apr 04 '23 08:04 Ionys320