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

feat: refactor with typescript to support esm and cjs both #51

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": [
"eslint-config-egg/typescript",
"eslint-config-egg/lib/rules/enforce-node-prefix"
],
"rules": {
"@typescript-eslint/ban-ts-comment": "off"
}
}
8 changes: 0 additions & 8 deletions .eslintrc.js

This file was deleted.

2 changes: 1 addition & 1 deletion .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ jobs:
uses: artusjs/github-actions/.github/workflows/node-test.yml@v1
with:
os: 'ubuntu-latest'
version: '14, 16, 18'
version: '16, 18, 20'
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ node_modules
coverage
*.un~
*.sw*
.tshy*
dist/
13 changes: 5 additions & 8 deletions bin/detect-port.js → bin/detect-port.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
#!/usr/bin/env node

'use strict';

const pkg = require('../package');
import pkg from '../package.json';
import detectPort from '../src/detect-port.js';

const args = process.argv.slice(2);
let arg_0 = args[0];

if (arg_0 && [ '-v', '--version' ].includes(arg_0.toLowerCase())) {
console.log(pkg.version);

process.exit(0);
}

Expand All @@ -21,18 +21,15 @@ const removeByValue = (arr, val) => {
}
};

const main = require('..');

const port = parseInt(arg_0, 10);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Number.parseInt for parsing integers to avoid global scope pollution.

- const port = parseInt(arg_0, 10);
+ const port = Number.parseInt(arg_0, 10);

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const port = parseInt(arg_0, 10);
const port = Number.parseInt(arg_0, 10);

const isVerbose = args.includes('--verbose');

removeByValue(args, '--verbose');
arg_0 = args[0];

if (!arg_0) {
const random = Math.floor(9000 + Math.random() * (65535 - 9000));

main(random, (err, port) => {
detectPort(random, (err, port) => {
if (isVerbose) {
if (err) {
console.log(`get available port failed with ${err}`);
Expand Down Expand Up @@ -61,7 +58,7 @@ if (!arg_0) {
console.log(` ${pkg.homepage}`);
console.log();
} else {
main(port, (err, _port) => {
detectPort(port, (err, _port) => {
if (isVerbose) {
if (err) {
console.log(`get available port failed with ${err}`);
Expand Down
4 changes: 0 additions & 4 deletions index.js

This file was deleted.

32 changes: 0 additions & 32 deletions lib/wait-port.js

This file was deleted.

58 changes: 42 additions & 16 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"detect": "./bin/detect-port.js",
"detect-port": "./bin/detect-port.js"
},
"main": "index.js",
"main": "./dist/commonjs/index.js",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The output indicates that the build scripts in package.json do not explicitly mention generating the ./dist/commonjs/index.js file. This suggests that the build process might not be correctly configured to produce the expected output file.

  • The prepublishOnly script runs tsc, tshy, and tshy-after, but none of these commands explicitly mention generating the ./dist/commonjs/index.js file.
  • The tsc script compiles ./bin/detect-port.ts, which does not align with the expected output path.

To resolve this, you should ensure that the build process correctly generates the ./dist/commonjs/index.js file. This might involve updating the TypeScript configuration or the build scripts.

Analysis chain

Ensure the "main" entry points to the correct compiled output.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the main entry points to the correct compiled output.

# Test: Check if the main entry exists.
test -f ./dist/commonjs/index.js

Length of output: 34



Script:

#!/bin/bash
# Description: Verify if the build scripts in package.json are configured to generate the expected output file.

# Extract the build scripts from package.json
jq '.scripts' package.json

# Check if the expected output file is mentioned in the build configurations
grep -r './dist/commonjs/index.js' .

Length of output: 539

"files": [
"bin",
"lib",
Expand All @@ -21,28 +21,54 @@
"url": "git://github.com/node-modules/detect-port.git"
},
"dependencies": {
"address": "^1.0.1",
"debug": "4"
"address": "^2.0.1"
},
"devDependencies": {
"command-line-test": "1",
"egg-bin": "^5.2.0",
"eslint": "^8.23.1",
"eslint-config-egg": "^12.0.0",
"git-contributor": "1",
"mm": "^2.1.0",
"pedding": "^1.1.0",
"power-assert": "^1.6.1"
"@eggjs/tsconfig": "^1.3.3",
"@types/node": "^20.9.0",
"c8": "^8.0.1",
"eslint": "^8.52.0",
"eslint-config-egg": "^13.0.0",
"execa": "^8.0.1",
"git-contributor": "^2.1.5",
"strip-ansi": "^7.1.0",
"ts-node": "^10.9.1",
"tshy": "^1.8.0",
"tshy-after": "^1.0.0",
"tsx": "^4.1.2",
"typescript": "^5.2.2"
},
"scripts": {
"test": "egg-bin test",
"ci": "npm run lint && egg-bin cov",
"lint": "eslint .",
"contributor": "git-contributor"
"test": "npm run lint -- --fix && tsx --test test/*.test.ts",
"lint": "eslint src test --ext ts",
"ci": "npm run lint && npm run cov && npm run prepublishOnly",
"contributor": "git-contributor",
"prepublishOnly": "npm run tsc && tshy && tshy-after",
"tsc": "tsc ./bin/detect-port.ts --resolveJsonModule --esModuleInterop",
"cov": "c8 npm test"
},
"engines": {
"node": ">= 4.0.0"
},
"homepage": "https://github.com/node-modules/detect-port",
"license": "MIT"
"license": "MIT",
"tshy": {
"exports": {
".": "./src/index.ts"
}
},
"exports": {
".": {
"import": {
"types": "./dist/esm/index.d.ts",
"default": "./dist/esm/index.js"
},
"require": {
"types": "./dist/commonjs/index.d.ts",
"default": "./dist/commonjs/index.js"
}
}
},
"types": "./dist/commonjs/index.d.ts",
"type": "module"
}
59 changes: 36 additions & 23 deletions lib/detect-port.js → src/detect-port.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,35 @@
'use strict';
import net from 'node:net';
import { ip } from 'address';
import { debuglog } from 'node:util';

const net = require('net');
const address = require('address');
const debug = require('debug')('detect-port');
const debug = debuglog('detect-port');

module.exports = (port, callback) => {
let hostname = '';
type DetectPortCallback = (err: Error | null, port: number) => void;

if (typeof port === 'object' && port) {
interface PortConfig {
port?: number | string;
hostname?: string | undefined;
callback?: DetectPortCallback;
}

export default function detectPort(port?: number | PortConfig | string): Promise<number>;
export default function detectPort(callback: DetectPortCallback): void;
export default function detectPort(port: number | PortConfig | string | undefined, callback: DetectPortCallback): void;
export default function detectPort(port?: number | string | PortConfig | DetectPortCallback, callback?: DetectPortCallback) {
let hostname: string | undefined = '';

if (port && typeof port === 'object') {
hostname = port.hostname;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid reassigning function parameters as it can lead to unexpected behavior and makes the code harder to understand.

- port = parseInt(port as unknown as string) || 0;
+ const parsedPort = Number.parseInt(port as unknown as string) || 0;

Also applies to: 28-28, 32-32, 23-23, 27-27, 32-32, 51-51, 54-54, 55-55, 128-128


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if (port && typeof port === 'object') {
if (port && typeof port === 'object') {
const parsedPort = Number.parseInt(port as unknown as string) || 0;

callback = port.callback;
port = port.port;
} else {
if (typeof port === 'function') {
callback = port;
port = null;
port = void 0;
}
}

port = parseInt(port) || 0;
port = parseInt(port as unknown as string) || 0;
let maxPort = port + 10;
if (maxPort > 65535) {
maxPort = 65535;
Expand All @@ -29,13 +40,13 @@ module.exports = (port, callback) => {
}
// promise
return new Promise(resolve => {
tryListen(port, maxPort, hostname, (_, realPort) => {
tryListen(port as number, maxPort, hostname, (_, realPort) => {
resolve(realPort);
});
});
};
}

function tryListen(port, maxPort, hostname, callback) {
function tryListen(port: number, maxPort: number, hostname: string | undefined, callback: (...args: any[]) => void) {
function handleError() {
port++;
if (port >= maxPort) {
Expand All @@ -50,7 +61,7 @@ function tryListen(port, maxPort, hostname, callback) {
if (hostname) {
listen(port, hostname, (err, realPort) => {
if (err) {
if (err.code === 'EADDRNOTAVAIL') {
if ((err as any).code === 'EADDRNOTAVAIL') {
return callback(new Error('the ip that is not unknown on the machine'));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify a more precise type than any for error handling.

- if ((err as any).code === 'EADDRNOTAVAIL') {
+ if ((err as NodeJS.ErrnoException).code === 'EADDRNOTAVAIL') {

Also applies to: 94-94, 119-119


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if ((err as any).code === 'EADDRNOTAVAIL') {
if ((err as NodeJS.ErrnoException).code === 'EADDRNOTAVAIL') {

}
return handleError();
Expand All @@ -60,34 +71,34 @@ function tryListen(port, maxPort, hostname, callback) {
});
} else {
// 1. check null
listen(port, null, (err, realPort) => {
listen(port, void 0, (err, realPort) => {
// ignore random listening
if (port === 0) {
return callback(err, realPort);
}

if (err) {
return handleError(err);
return handleError();
}

// 2. check 0.0.0.0
listen(port, '0.0.0.0', err => {
if (err) {
return handleError(err);
return handleError();
}

// 3. check localhost
listen(port, 'localhost', err => {
// if localhost refer to the ip that is not unkonwn on the machine, you will see the error EADDRNOTAVAIL
// https://stackoverflow.com/questions/10809740/listen-eaddrnotavail-error-in-node-js
if (err && err.code !== 'EADDRNOTAVAIL') {
return handleError(err);
if (err && (err as any).code !== 'EADDRNOTAVAIL') {
return handleError();
}

// 4. check current ip
listen(port, address.ip(), (err, realPort) => {
listen(port, ip(), (err, realPort) => {
if (err) {
return handleError(err);
return handleError();
}

callback(null, realPort);
Expand All @@ -98,21 +109,23 @@ function tryListen(port, maxPort, hostname, callback) {
}
}

function listen(port, hostname, callback) {
function listen(port: number, hostname: string | undefined, callback: (...args: any[]) => void) {
const server = new net.Server();

server.on('error', err => {
debug('listen %s:%s error: %s', hostname, port, err);
server.close();
if (err.code === 'ENOTFOUND') {

if ((err as any).code === 'ENOTFOUND') {
debug('ignore dns ENOTFOUND error, get free %s:%s', hostname, port);
return callback(null, port);
}

return callback(err);
});

server.listen(port, hostname, () => {
port = server.address().port;
port = (server.address() as net.AddressInfo).port;
server.close();
debug('get free %s:%s', hostname, port);
return callback(null, port);
Expand Down
6 changes: 6 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import detectPort from './detect-port.js';
import waitPort from './wait-port.js';

export default detectPort;

export { waitPort };
35 changes: 35 additions & 0 deletions src/wait-port.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { debuglog } from 'node:util';
import detectPort from './detect-port.js';

const debug = debuglog('wait-port');

const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms));

export interface WaitPortOptions {
retryInterval?: number;
retries?: number;
}

export default async function waitPort(port: number, options: WaitPortOptions = {}) {
const { retryInterval = 1000, retries = Infinity } = options;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Number.Infinity for clarity and consistency.

- const { retryInterval = 1000, retries = Infinity } = options;
+ const { retryInterval = 1000, retries = Number.Infinity } = options;

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const { retryInterval = 1000, retries = Infinity } = options;
const { retryInterval = 1000, retries = Number.Infinity } = options;

let count = 1;

async function loop() {
debug('retries', retries, 'count', count);
if (count > retries) {
const err = new Error('retries exceeded');
(err as any).retries = retries;
(err as any).count = count;
Comment on lines +21 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using any type; specify a more appropriate type.

- (err as any).retries = retries;
- (err as any).count = count;
+ (err as CustomError).retries = retries; // Assuming CustomError is defined elsewhere
+ (err as CustomError).count = count;

Committable suggestion was skipped due low confidence.

throw err;
}
count++;
const freePort = await detectPort(port);
if (freePort === port) {
await sleep(retryInterval);
return loop();
}
return true;
}

return await loop();
}
Loading
Loading