http-party / node-portfinder

A simple tool to find an open port or domain socket on the current machine
https://github.com/http-party/node-portfinder
Other
887 stars 95 forks source link

why mkdirp dependecy? #131

Closed stefanocudini closed 2 years ago

stefanocudini commented 2 years ago

I would like to include this library in various applications, but this dependency seems to me to make little sense

eriktrom commented 2 years ago

We could replace https://github.com/http-party/node-portfinder/blame/38cf0185a18c1ecb2c2590ae91f0a251b612ab15/lib/portfinder.js#L328 with the node builtin filesystem module.

That code is for creating sockets.

You'll recognize more pieces of that code here: https://github.com/http-party/node-portfinder/blame/38cf0185a18c1ecb2c2590ae91f0a251b612ab15/lib/portfinder.js#L384

Feel free to peruse the code, it's 11 years old and I didn't write it, not that that really matters at all. Here is the original commit: https://github.com/http-party/node-portfinder/commit/f1cd7aeab8e1962a28fbda69255a3588e83add70

Submit a PR that uses the builtin node js mkdir: https://nodejs.org/api/fs.html#fsmkdirsyncpath-options and I'll merge it :)

eriktrom commented 2 years ago

Releasing a new version that will have no mkdirp installed, will use node directly, this week.

FYI

MasterOdin commented 2 years ago

Are you looking to replace mkdirp with fs.mkdir and make use of the recursive flag added in v10 LTS (and thus breaking support for anything < 10), or doing the recursion by hand in fs.mkdir?

jimmywarting commented 2 years ago

I'm hoping for a breaking change... v14 is the LTS right now...

eriktrom commented 2 years ago

mkdirp recursively created directories, so just to clarify @MasterOdin in your comment:

(and thus breaking support for anything < 10), or doing the recursion by hand in fs.mkdir?

we do want to recursively create a folder structure - at least that is how the original code was written, and hence its use of mkdirp, which 11 years ago, was how mkdir -p was popularly done in nodejs.

Let me know if I missed something here, thanks.

see #134 for its removal and use of native node api's for mkdir -p

Closed by #134

MasterOdin commented 2 years ago

I created #135 (as there was no test with the recursive folder creation) to help demonstrate my point where if you wish to support node < 10.12, then you'll need to either use mkdirp or do the recursion yourself as fs.mkdir does not support doing it in those old versions of node, but if you drop the old versions (which I'd argue you should), then fs.mkdir works fine as a replacement.

eriktrom commented 2 years ago

closing in favor of use of native builtins from the nodejs' fs module: #134 #135 (collectively, #134 includes all changes for modernization, iteration 1, fwiw)

eriktrom commented 2 years ago

@stefanocudini - to use this library, ideally wrap it in a cli, or you can also from your shell:

eval("require('./lib/portfinder').getPort({ host: '127.0.0.1', port: '8889'}, (err, port) => { console.log(port); })");
eriktrom commented 2 years ago

see #137 for current status, fyi