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(serve): env var DENO_SERVE_ADDRESS for configuring default listener address #24382

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

losfair
Copy link
Member

@losfair losfair commented Jul 1, 2024

This patch adds support for configuring a default listen address for Deno.serve() with the DENO_SERVE_ADDRESS environment variable. If none of options.{hostname,port,path} is set, the listener will attempt to parse DENO_SERVE_ADDRESS and listen on the specified endpoint.

The listen address is specified in a network/address format. For example:

  • tcp/0.0.0.0:8080
  • unix//var/run/http.sock

@losfair losfair changed the title feat(serve): env var DENO_SERVE_ADDRESS for configuring default lis… feat(serve): env var DENO_SERVE_ADDRESS for configuring default listener address Jul 1, 2024
@bartlomieju bartlomieju self-requested a review July 1, 2024 16:20
@losfair losfair marked this pull request as ready for review July 1, 2024 19:28
Comment on lines +598 to +602
const canOverrideOptions = !ObjectHasOwn(options, "path")
&& !ObjectHasOwn(options, "hostname")
&& !ObjectHasOwn(options, "port");
const env = Deno.permissions.querySync({ name: "env", variable: "DENO_SERVE_ADDRESS" }).state === "granted"
&& Deno.env.get("DENO_SERVE_ADDRESS");
Copy link
Member

Choose a reason for hiding this comment

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

This looks very funky. We are already forwarding host and port in

// Used by `deno serve`
pub serve_port: Option<u16>,
pub serve_host: Option<String>,
, I suggest that instead of doing all of this parsing in JS, we do that in Rust and only for deno serve subcommand. Otherwise this code will be applied to every single Deno.serve() call.

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

Successfully merging this pull request may close these issues.

None yet

2 participants