Roadmap to V1.0

[mathe42]

Version 1.0.0

• [x] stable api (the new syntax is different but good to adopt) internal changes might follow!
• [x] is worker and pool syntax good? With new syntax yes.
• [x] better documentation
  • [x] rewrite
  • [x] what is the stable api surface
  • [x] add tsdoc examples
• [x] testing (at least a e2e test of all features) [done with a mix of https://www.npmjs.com/package/fake-smtp-server and https://mailtrap.io/ I think] I consider this done for now but after 1.0 we need more!
• [x] make general class / function with same api for worker, pool, client (done with class)
• [x] sendConfig preprocessing (filter etc)
• [x] extract SmtpClient que to external class
• [x] rename SMTPHandler
• [x] unref fork (requested via github support)
• [x] TSDOC comments

[xCykrix]

Hey! Just wanted to leave a little feedback based on the roadmap.

In regards to "Stable API" and "Is the worker and pool syntax good?" I would potentially recommend trying to consolidate classes. Have the "SmtpClient" class that you configure based on the options you give in so that it is a consistent API if you want to change to pooling later as a developer with growth.

// Pseudo-code. The typings are included where actual values would be.

const smtp = new SmtpClient({
  debug?: boolean, // default false
  connection: {
    hostname: string,
    port: number,
    username: string,
    password: string,
    secure: boolean, // If TLS should be used, and would trigger logic similar to connectTLS so it can be deprecated?
  },
  pool?: { // Set the entire client as a "pool" but default to 1/1 to act as a single client.
    min?: number, // The minimum number of active clients. Default: 1
    max?: number, // The maximum number of active clients. Default: 1
    timeout?: number, // The timeout in ms for clients above the minimum to disconnect with no activity. Default 60000? 120000?
  },
});

await smtp.connect(); // No options here, just trigger the connection. (Can probably deprecate connectTLS?; You could soft deprecate it by mapping the connection values back to the options to allow time to change for v1.)

await smtp.send({ ... });

Just my two cents. Very interested in where this project will go as it seems promising.

[mathe42]

I agree and your options look quite good. (I have some other names in mind but more or less the same).

But as the project I want to use this for has some other work (a custom api with websockets). Not shure if we need a class or if a function createSMTPClient(...) would be better...

[xCykrix]

I mean, you could 100% do both. Having a fully stable class, it would be trivial to export a function that wraps around the creation logic of the class for easier initialization but also gives the option for people to use the class.

I tend to make "module" classes that hold all the logic my application/tool uses for each dependency if the dependency is used in multiple places. Being able to just initialize the class simplifies that process slightly and just looks more concise.

[mathe42]

WIP options.

interface ClientOptions {
  debug?: {
    /**
     * USE WITH COUTION AS THIS WILL LOG YOUR USERDATA AND ALL MAIL CONTENT TO STDOUT!
     * @default false
     */
    log?: boolean
    /**
     * USE WITH COUTION AS THIS WILL POSIBLY EXPOSE YOUR USERDATA AND ALL MAIL CONTENT TO ATTACKERS!
     * @default false
     */
    allowUnsecure?: boolean,
    /**
     * USE WITH COUTION AS THIS COULD INTODRUCE BUGS
     * 
     * This option is mainly to allow while debuging to exclude some possible problem surfaces with the encoding
     * @default false
     */
    encodeLB?: boolean
  },
  connection: {
    hostname: string,
    /**
     * For TLS the default port is 465 else 25.
     * @default 25 or 465
     */
    port?: number
    auth?: {
      username: string
      password: string
    }
    /**
     * Set this to `true` to connect via SSL / TLS if set to `false` STARTTLS is used.
     * Only if `allowUnsecure` is used userdata or mail content could be exposed!
     * 
     * @default false
     */
    tls?: boolean
  },
  pool?: {
    // As I currently use roundrobin to use the worker min, max are not that great. And if you don't use a connection for days I think you should close it...
    /**
     * Number of Workers
     * @default 2
     */
    size?: number
    /**
     * Time the connection has to be idle to be closed. (in ms)
     * If a value > 1h is set it will be set to 1h
     * @default 60000
     */
    timeout?: number 
  }
}

[xCykrix]

Looks good. Makes sense for the size since you use round robin. I haven't dug much into the code so I didn't know the style of queuing you used.

[mathe42]

I worked a lot on this today and yesterday and I basicly completely restructured the code.

I want to add at least for a pool of size 1 a end to end test before releasing 1.0 more can follow later.