Thanks for the PR.

I think we also need to wait for Deploy to support Deno.serve before landing this

Perhaps, std/http/file_server.ts should also be adjusted to use Deno.serve() in this PR.

What version of std should this be removed by?

I found a few files that use serve or similar and I'm not sure if it makes sense for them to stick around:

• http/bench.ts - seems like just a quick file to bench the http server, doesn't really make sense to stick around AFAIK
• http/testdata/simple_https_server.ts - seems to have been used at one point in a test but no longer is referenced anywhere in the codebase. Also doesn't work right now at all since it imports something that doesn't exist.
• http/testdata/simple_server.ts - seems to have been used at one point in a test but is no longer referenced anywhere in the codebase

Should these be removed? I've updated them to use Deno.serve for now.

Should these be removed? I've updated them to use Deno.serve for now.

I think so. The points you've raised seem to justify removing them.

Should these be removed?

They look ok to remove to me too.

Deno.serve just landed in deploy. Should now be fine to merge after a review.

There are snippets that contain the use of the std/http/server, which I think should be replaced with the use of Deno.serve(). E.g. https://deno.land/std@0.193.0/http/server_sent_event.ts

I'm wondering if we should also deprecate serveListener and the Server class?

I think we should.

I'm wondering if we should also deprecate serveListener and the Server class?

I think that's another independent topic. The use case like serving HTTP over unix domain socket listener (Deno.listen({ path: "/foo/bar.sock", transport: "unix" })) is not covered by Deno.serve.

I think that's another independent topic. The use case like serving HTTP over unix domain socket listener (Deno.listen({ path: "/foo/bar.sock", transport: "unix" })) is not covered by Deno.serve.

Ah, didn't know that. Reverting the deprecations.

The trouble I'm having is that we should definitely remove some types before 1.0 and replace them with ones that make more sense. For instance

export interface ServeInit extends Partial<Deno.ListenOptions> {
  signal?: AbortSignal;
  onError?: (error: unknown) => Response | Promise<Response>;
  onListen?: (params: { hostname: string; port: number }) => void;
}

is used for serve and serveListener. With serve being dropped, this type will only matter for serveListener. serveListener uses the type as Omit<ServeInit, "port" | "hostname">. It doesn't make sense for this type to extend Deno.ListenOptions if the only reference to it in the future will specifically omit all of the types that Deno.ListenOptions provides.

I think that's another independent topic. The use case like serving HTTP over unix domain socket listener (Deno.listen({ path: "/foo/bar.sock", transport: "unix" })) is not covered by Deno.serve.

According the API reference, Deno.listen() doesn't support UNIX sockets. Neither do serveListener() and Server. Is this right?

@iuioiua

According the API reference, Deno.listen() doesn't support UNIX sockets. Neither do serveListener() and Server. Is this right?

That should be visible with Show Unstable API checked

I'm unsure if we should land this in it's current state with #3381 (comment) still remaining a question. Any thoughts?

Ah, ok. I missed that.

With serve being dropped, this type will only matter for serveListener. serveListener uses the type as Omit<ServeInit, "port" | "hostname">. It doesn't make sense for this type to extend Deno.ListenOptions if the only reference to it in the future will specifically omit all of the types that Deno.ListenOptions provides.

I think replacing Omit<ServeInit, "port" | "hostname"> with its own interface (such as ServeListenerOptions) with only necessary props copied from ServeInit makes more sense.

I think replacing Omit<ServeInit, "port" | "hostname"> with its own interface (such as ServeListenerOptions) with only necessary props copied from ServeInit makes more sense.

Seems reasonable, going to update the PR.

LGTM now.

Tested this branch's version of file_server https://raw.githubusercontent.com/denoland/deno_std/b2a0642521f5323a9bd9e772fafc3f006ac7e936/http/file_server.ts on Deploy, and it seemed working https://small-hawk-66-j5jx0nq05yh0.deno.dev/

I think this is ready to land.

Thank you for your contribution!