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

suggestion: remove getAvailablePort() from @std/net #5364

Closed
iuioiua opened this issue Jul 9, 2024 · 2 comments
Closed

suggestion: remove getAvailablePort() from @std/net #5364

iuioiua opened this issue Jul 9, 2024 · 2 comments
Labels
net suggestion a suggestion yet to be agreed

Comments

@iuioiua
Copy link
Contributor

iuioiua commented Jul 9, 2024

I suggest removing getAvailablePort() from @std/net.

Since implementation, Deno.HttpServer.addr.port became available in Deno 1.43. This satisfies the original problem that getAvailablePort() aimed to solve. Example use:

// main.ts
export const port = Deno.serve({ port: 0 }, (_req) => new Response("Hello, world")).addr.port;
// test.ts
import { port } from "./main.ts";
// ...

Secondly, at least according to this GitHub search, it's easily replaced with port: 0 (documentation to be added in denoland/deno#24475).

- import { getAvailablePort } from "@std/net";

Deno.serve({
- port: getAvailablePort(),
  port: 0,
}, (_req) => new Response("Hello, world"));

Lastly, the function is susceptible to a race condition (a port that is deemed available becomes unavailable in between calling getAvailablePort() and the following Deno.serve(), and introduces a dependency when the same functionality is already built into the runtime.

Happy to hear thoughts.

CC @harrysolovay @jollytoad @bcheidemann @dominiq007 (I found your repos used this function)

@iuioiua iuioiua added suggestion a suggestion yet to be agreed net labels Jul 9, 2024
@iuioiua
Copy link
Contributor Author

iuioiua commented Jul 9, 2024

Edit: A reasonable edge was found in the following code. We'll now instead add documentation advising against the use of this function, in favor of using port: 0. https://github.com/denoland/deno_std/blob/1f26aa503b083ccdf08113f663a176b98d012d59/http/file_server_test.ts#L991-L1040

@iuioiua iuioiua closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2024
@jollytoad
Copy link
Contributor

Also the preferredPort feature is not available natively in Deno, yet can be quite useful during development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net suggestion a suggestion yet to be agreed
Projects
None yet
Development

No branches or pull requests

2 participants