Docs on http.Server.listen() need clarifications on accepted args

[claudiopro]

• Version: v4-v8
• Platform: macOS 10.12.6
• Subsystem: http, net

This is in relation to facebook/flow#1684

While Flow typing a module that uses http.Server.listen(), I received an error for the idiomatic form widely used in Node projects:

const server = http.createServer();
server.listen(8080, function listening() {});

Flow declares the types of http.Server.listen() like this:

declare module "http" {
  declare class Server extends net$Server {
    listen(port: number, hostname?: string, backlog?: number, callback?: Function): Server;
    listen(path: string, callback?: Function): Server;
    listen(handle: Object, callback?: Function): Server;

which adheres to the documentation of the http module.

Since the net module normalizes the arguments and Node itself uses the shorthand forms in the docs on the same page (Event: 'connect', Event: 'upgrade'), I suggest to amend the documentation explaining the shorthand forms are both supported:

listen(port, callback)
listen(port, hostname, callback)

Thoughts?

[apapirovski]

I could be misunderstanding but doesn't the http module already document this as such in server.listen([port][, hostname][, backlog][, callback])? Meaning any of these arguments can be safely omitted.

[mscdex]

@apapirovski is right, any arguments inside brackets in docs are indicating optional parameters and server.listen() already shows that, so I don't think anything needs to be changed on our end. It seems like a bug/issue with Flow.

[claudiopro]

@mscdex @apapirovski thanks for clearing my misunderstanding, effectively the behavior of Flow is inconsistent with what server.listen() actually accepts, forcing the implementor to pass literal undefined in place of the omitted argument(s).

I'll bring it back to the Flow team for discussion.

[joyeecheung]

Closing because the documentation is clear enough.

[claudiopro]

@joyeecheung I'd ask you to leave this open as I believe there are still some inconsistencies.

The docs don't specify what is the expected behavior when encountering the signature with one argument server.listen(Number), or with two arguments, server.listen(Number, Function):

// This is interpreted as a port number
server.listen(8080);
// This is interpreted as a port number
server.listen(8080, function callback() {});

which consider the first Number as port, while the notation server.listen([port][, hostname][, backlog][, callback]) suggests that you could pass a backlog but no port.

Additionally, the signature server.listen(String, Function) can match both server.listen(path[, callback]) and server.listen([port][, hostname][, backlog][, callback]) with only hostname and callback set.

I would like to clarify the expected behavior 😄

[joyeecheung]

while the notation server.listen([port][, hostname][, backlog][, callback]) suggests that you could pass a backlog but no port.

I would interprete this as "if there is any argument, and the first one is a number ,then the first one is a port" though, I am not sure if this optional argument syntax has any formal standard, but to me the previous arguments can't be optional like that (server.listen(backlog[, callback]) does not match server.listen([port][, hostname][, backlog][, callback]) because the [, part is there for a reason.)

Additionally, the signature server.listen(String, Function) can match both server.listen(path[, callback]) and server.listen([port][, hostname][, backlog][, callback]) with only hostname and callback set.

I can't really see how the second one can be matched either, for the same reason above.

[joyeecheung]

@claudiopro OK I think there is another way to make it clearer, although that requires a lot of brackets...

server.listen([[[port[, hostname[, backlog]]][, callback])