search

leafac / kill-the-newsletter

Convert email newsletters into Atom feeds
https://kill-the-newsletter.com
MIT License
2.31k stars 113 forks source link

Support Node 18 #67

Closed themaxdavitt closed 1 year ago

themaxdavitt commented 1 year ago

Was running into errors running the test on Node 18, e.g.:

TypeError: Cannot set property closed of #<Writable> which has only a getter
    at new SMTPStream (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-stream.js:34:21)
    at new SMTPConnection (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-connection.js:55:24)
    at SMTPServer.connect (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-server.js:93:26)
    at /Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-server.js:84:26
    at Immediate.<anonymous> (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-server.js:345:39)
    at processImmediate (node:internal/timers:471:21)
TypeError: Cannot set property errored of [object Object] which has only a getter
    at new MessageSplitter (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/mailsplit/lib/message-splitter.js:28:22)
    at new MailParser (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/mailparser/lib/mail-parser.js:132:25)
    at Object.<anonymous>.module.exports [as simpleParser] (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/mailparser/lib/simple-parser.js:29:18)
    at SMTPServer.onData (/Users/themaxdavitt/Projects/kill-the-newsletter/source/index.ts:437:40)
    at SMTPConnection.handler_DATA (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-connection.js:1307:22)
    at SMTPConnection._onCommand (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-connection.js:497:17)
    at SMTPStream.SMTPConnection._parser.oncommand (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-connection.js:58:52)
    at readLine (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-stream.js:128:22)
    at SMTPStream._write (/Users/themaxdavitt/Projects/kill-the-newsletter/node_modules/smtp-server/lib/smtp-stream.js:132:13)
    at writeOrBuffer (node:internal/streams/writable:392:12)

Turns out that it added some properties to stream.Writable and stream.Readable that overlapped with property names in derived classes in two dependencies: smtp-server and mailparser. After updating those dependencies to versions that include the (linked) commits fixing the issues, which also happen to be the latest versions of them, the test now works for me.

leafac commented 1 year ago

Thanks for the pull request and for the information. I’m looking into updating a bunch of things about Kill the Newsletter! and I’ll merge this soon 😁

themaxdavitt commented 1 year ago

I haven't tested it but I looked at the package.json entries for those two dependencies on the main branch and I think they should be compatible with Node 18 now. Thanks for fixing this!