search

eleith / emailjs

html emails and attachments to any smtp server with nodejs
MIT License
2.19k stars 230 forks source link

inline address parsing #259

Closed zackschuster closed 4 years ago

zackschuster commented 4 years ago

i'm not happy with the current types situation re: addressparser. if there were a maintained @types/addressparser package i would happily add it to dependencies & call it a day. unfortunately we have to ask people to modify their typeRoots configuration instead, and that's not the most friendly thing in the world.

i'm thinking of inlining the code from emailjs-addressparser like i did for emailjs-mime-codec, though the api is different so it would require some internal refactoring.

eleith commented 4 years ago

talking out loud here, thinking of a few options

1) upstream maintainers of addressparser/mim-codec accept PR converting projects to typescript 2) adding a @types ourselves and contributing to https://github.com/DefinitelyTyped/DefinitelyTyped 3) forking repos, converting to typescript and maintaining them separately 4) inlining them

they all come with their flavor of tradeoffs. in general, i'ld like for the footprint of this project to not grow though... (on the other hand, each of the modules in questions do fit very well into one small file that is easily unit tested).

your thoughts?

zackschuster commented 4 years ago

i'll reply point-by-point or i might end up all over the place :D

1) i'd agree & would prefer to share the love if the emailjs libraries were ongoing efforts, but they appear unmaintained, which is why i feel comfortable "plundering" the code instead. 2) i've never done this before and have no idea what the maintenance burden would be. we'd also likely want to coordinate with nodemailer somehow since they "own" the addressparser package. 3) multi-repo setups are a not-insignificant cognitive/maintenance burden of their own imo. i'd rather not do this haha. 4) we would drop to zero dependencies, which makes me shiver with delight. the code itself is a single file, so it would be relatively easy to inline. (addressparser is a single file too but i feel weird about plundering that code since it's maintained).

actually, i wrote all this up & then looked over emailjs-addressparser & addressparser more carefully — they're nearly the same & in fact i think the same person worked on both; an early version of the emailjs-addressparser code shows a copyright for Andris Reinman who appears to maintain addressparser.

i'll take a leap & ping @andris9 — hopefully they would like to chime in :)

andris9 commented 4 years ago

The main difference between addressparser and emailjs-addressparser is that all emailjs-* modules were mainly meant for browser environment while nodemailer code is, as the name implies, fore nodejs only. emailjs-* modules were built for a company I worked for and that went bankrupt already in 2015. Some of my co-workers tried to maintain these modules even afterwards but they have other things going on by now. In any case you are welcomed to “plunder” anything you find worthwhile from these modules.

zackschuster commented 4 years ago

thanks for the background, @andris9 :) incidentally, i find it a shame that browsers never really allowed client programs to make full-fledged tls connections, even in a limited, always-explicit-consent fashion. alas. what could've been!

all that being said, i feel much more confident in going for inlining the code.

andris9 commented 4 years ago

@zackschuster Whiteout email client was running as a Chrome App what basically meant a webpage with some additional privileged APIs like TCP access. At one point we moved the entire app into a webpage with a websocket-based proxy (all TLS handling was done in browser and the websocket server was just a dumb proxy moving TCP packets between the app and destination email server). Basically we were able to get the app running on any platform as long as we were able to somehow get the TCP connections working (Chrome App, web page with websocket proxy, FirefoxOS, iOS/Android with Phonegap, Windows Universal App etc.)

zackschuster commented 4 years ago

@andris9 wow! that's close to my ideal model — i'd minus the websocket TCP forwarder, but i'm sure there's a "good" reason for it existing (CORS?). i would be VERY interested in recreating that work here if browser vendors ever decide to officially support the necessary APIs.