Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when calling listen on Node's http server #1684

Open
ntharim opened this issue Apr 19, 2016 · 6 comments
Open

Error when calling listen on Node's http server #1684

ntharim opened this issue Apr 19, 2016 · 6 comments
Labels
Library definitions Issues or pull requests about core library definitions

Comments

@ntharim
Copy link

ntharim commented Apr 19, 2016

flow has support for Node's http module. However calling listen on a Server instance causes an error that shouldn't exist. Given the following code:

import http from 'http';

const server = http.createServer(() => {});
server.listen(3000, () => {});

The error is:

call of method `listen`
Function cannot be called on any member of intersection type

You can see this on tryflow.org:
http://tryflow.org/#6b0a9f9d52c7320c9e9de182fa081c55

@yinanfang
Copy link

I have the same problem as well! Why there's no one looking at this problem? @nthtran

@bakotaco
Copy link
Contributor

@nthtran FYI the link you provided (http://tryflow.org/#6b0a9f9d52c7320c9e9de182fa081c55) does not illustrate the error (anymore)

@bakotaco
Copy link
Contributor

the issue seems to be that the http.Server.listen method is defined as follows:

listen(port: number, hostname?: string, backlog?: number, callback?: Function): Server;

So if there is a second argument provided to listen flow will assume this is the hostname parameter, however the provided callback Function does not match the expected string type. There is also a second definition:

listen(path: string, callback?: Function): Server;

Which does expect the second parameter to be a Function, but does not match as the first provided argument is a number.

You could 'solve' this by explicitly provided undefined as arguments for the listen call, e.g.:

server.listen(3000, undefined, undefined, () => {});

@yinanfang
Copy link

@bakotaco That solved my problem! THANKS!
Most of the express documentations are doing something like server.listen(3000, () => {}); and I just overlooked the [backlog] there.

@mroch mroch added the Library definitions Issues or pull requests about core library definitions label May 31, 2016
@jedwards1211
Copy link
Contributor

jedwards1211 commented Nov 11, 2016

@mroch ugly as hell but this definition seems to work:

declare var listen:
  ((port?: number, hostname?: string, backlog?: number, callback?: Function) => void) &
  ((port?: number, hostname?: string, callback?: Function) => void) &
  ((port?: number, backlog?: number, callback?: Function) => void) &
  ((port?: number, callback?: Function) => void) &
  ((hostname?: string, backlog?: number, callback?: Function) => void) &
  ((hostname?: string, callback?: Function) => void) &
  ((backlog?: number, callback?: Function) => void) &
  ((callback?: Function) => void)

The downside is that the error messages are loooooooong. Should I make a PR or no?

Alternatively, assuming that invalid arguments will just be ignored by the implementation, perhaps it would be best to change the signature to just Function for now

@jedwards1211
Copy link
Contributor

jedwards1211 commented Nov 11, 2016

@mroch proposal: use ?? for arguments that can be omitted even if following arguments are included. So for example a function with some optional arguments that requires a callback as last argument:

declare module fs {
  declare function writeFile(
    file: string | Buffer | number, 
    data: string | Buffer, 
    options??: Object | string, 
    callback?: (err: ?ErrnoError) => void
  ): void;
}

claudiopro pushed a commit to claudiopro/flow that referenced this issue Oct 19, 2017
…ions

Adds tests for the `http` and `https` modules. Improves the definitions of `server.listen()` to allow omitting intermediate arguments in the `listen(Number, String, Number, Function)` signature, and making all arguments optional. Addresses facebook#1684.
claudiopro pushed a commit to claudiopro/flow that referenced this issue Oct 19, 2017
Adds tests for the `http` and `https` modules. Improves the definitions of `server.listen()` to allow omitting intermediate arguments in the `listen(Number, String, Number, Function)` signature, and making all arguments optional. Addresses facebook#1684.
claudiopro pushed a commit to claudiopro/flow that referenced this issue Oct 19, 2017
Adds tests for the `http` and `https` modules. Improves the definitions of `server.listen()` to allow omitting intermediate arguments in the `listen(Number, String, Number, Function)` signature, and making all arguments optional. Addresses facebook#1684.
facebook-github-bot pushed a commit that referenced this issue Oct 19, 2017
Summary:
Adds tests for the `http` and `https` modules. Improves the definitions of `server.listen()` to allow omitting intermediate arguments in the `listen(Number, String, Number, Function)` signature, and making all arguments optional. Addresses #1684.

See also related discussion on the Node repo: nodejs/node#16300
Closes #5144

Reviewed By: calebmer

Differential Revision: D6098963

Pulled By: claudiopro

fbshipit-source-id: dae9911842a04fb33ce52491e6f38ddfcd62b8c7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Library definitions Issues or pull requests about core library definitions
Projects
None yet
Development

No branches or pull requests

5 participants