http: serve() should log where it is listening #1641
Conversation
http/server.ts
Outdated
| const server = new Server({ addr, handler }); | ||
|
|
||
| if (options?.signal) { | ||
| options.signal.onabort = () => server.close(); | ||
| } | ||
|
|
||
| console.log(`Listening on http://${addr}/`); |
There was a problem hiding this comment.
This should probably use the resolved address, rather than the original address.
|
people can use listenAndServe if they don't want the log message. This is a convenience function. |
I like the idea. So in that case we cancel the deprecation of |
|
Is there a precedence for including logging in a std module? This change implicitly assumes that the console is the right place to log. Std has a whole logging framework std/log because that might not be the case. Also what if you want a different log message, e.g. add your application name, log levels, etc. pp.? Baking a log message into the std module makes all of this impossible. What if you want no logging? Need to use an option or even another function to turn logging off? Imagine for a second this logging by default is true throughout the whole std library… Yikes! Also the motivating Deno Deploy examples should probably instead delete the “Listening on http://localhost:” logs because this URL makes sense only when running locally with Deno CLI, not when running on Deno Deploy, which has a different URL (not localhost!) and ignores the port. A much better place would be a note in the documentation on “Running locally” using the Deno CLI, mentioning that It seems to me a rather thoughtless optimization to reduce a given example code by 1 LOC. I advise to carefully reconsider this. |
|
Having thought about this again, I now feel it's not worth providing 2 different APIs with such subtle difference. If we have more features to add to the 'convenient' API, then that might worth it. However if the logging of that line is the only difference, I don't think it's worth it.. |
|
Reconsidered this again. |
reduces boilerplate in examples https://deno.com/deploy/docs/examples