-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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(ext/http): Add addr
to HttpServer
#23442
Conversation
addr
to HttpServer [WIP]addr
to HttpServer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question. Can you run deno_std
(and maybe fresh
) tests against this branch to see if there are any issues with new types?
cli/tsc/dts/lib.deno.ns.d.ts
Outdated
| ServeTlsOptions | ||
| (ServeTlsOptions & TlsCertifiedKeyOptions), | ||
handler: ServeHandler, | ||
): HttpServer; | ||
handler: ServeTlsHandler, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this specialized interface here? Feels to me like TLS should use ServeHandler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually will need this eventually as we have to pass the TLS handshake data in. I can push this off for now but we definitely need it (soon).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rolled back the ServeTlsHandler
for now, but left ServeTlsInit
in-place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to have! I thought we already had this.
Passed:
|
57ad536
to
a0b4644
Compare
Adds an
addr
field toHttpServer
to simplify the patternDeno.serve({ onListen({ port } => listenPort = port })
. This becomes:const server = Deno.serve({}); port = server.addr.port
.Changes:
serve
overloads to split TLS out (in preparation for landing a place for the TLS SNI information)addr
field toHttpServer
that matches theaddr
field of the correspondingDeno.Listener
s.