You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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.
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:
#185
#187
#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.
The text was updated successfully, but these errors were encountered: