Detect docker?

[OmgImAlexis]

This library either can't detect docker containers using ports or can't detect mongodb.

Ref: CImrie/mongomem#2 (comment)

[OmgImAlexis]

Looks like it can't detect docker. Changing the script to check port 3200 and then using docker run -d -p 3200:80 tutum/hello-world returns 3200 even though the port is now taken.

[sindresorhus]

This module uses port 0, which should give you an unused port. If it doesn't do that, it's better to open an issue on Node.js.

[OmgImAlexis]

This is a bug with the net package, right?

Changed the test.

diff --git a/test.js b/test.js
index 9e48c61..11abc86 100644
--- a/test.js
+++ b/test.js
@@ -17,7 +17,7 @@ test('port can be bound when promise resolves', async t => {
 });
 
 test('preferred port', async t => {
-	const desiredPort = 8080;
+	const desiredPort = 27017;
 	const port = await m(desiredPort);
 
 	t.is(port, desiredPort);

➜  get-port git:(master) ✗ yarn test -- --verbose
yarn test v0.24.6
$ xo && ava --verbose

  ✔ preferred port
  ✔ port can be bound when promise resolves
  ✔ preferred port unavailable

  3 tests passed [16:46:43]

✨  Done in 5.42s.
➜  get-port git:(master) ✗ docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES
➜  get-port git:(master) ✗ docker start mongodb
mongodb
➜  get-port git:(master) ✗ docker ps
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS                      NAMES
9f859e421e0e        mongo               "/entrypoint.sh mo..."   2 months ago        Up 1 second         0.0.0.0:27017->27017/tcp   mongodb
➜  get-port git:(master) ✗ yarn test -- --verbose
yarn test v0.24.6
$ xo && ava --verbose

  ✔ preferred port
  ✔ port can be bound when promise resolves
  ✔ preferred port unavailable

  3 tests passed [16:47:03]

✨  Done in 4.52s.

[sindresorhus]

This is a bug with the net package, right?

If it's a bug, yes. Note that it's not a package, but a core module.

[OmgImAlexis]

As per nodejs/node#14034 (comment) maybe use 0.0.0.0 for the host as a default so Docker container and other apps bound to 0.0.0.0 are detected.

[acostalima]

@OmgImAlexis as of v3.2.0 (9782a34), you can pass the host address as an option. The defaults are provided by Node which, IMO, should be kept that way.
@sindresorhus what do you think?

[OmgImAlexis]

@acostalima maybe a warning in the README needs to be added then since I would expect this package to check all ports not just those bound to localhost.

[acostalima]

@sindresorhus what do you think?

[sindresorhus]

I don’t think we should change the defaults, but a note in the readme makes sense.

[acostalima]

@OmgImAlexis would you like to open a PR?

[sindresorhus]

It now checks all local hosts by default: https://github.com/sindresorhus/get-port/releases/tag/v6.0.0