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

✨ add listen callback function #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fdemir
Copy link

@fdemir fdemir commented Jul 10, 2022

i didn't write some tests since we have no proper test integration. but it can be able to merge now.

@fdemir
Copy link
Author

fdemir commented Jul 10, 2022

closes #4

@fdemir fdemir changed the title ✨ add listen callback funciton ✨ add listen callback function Jul 15, 2022
@eladcandroid
Copy link

@mattreid1 Is it good enough to be merged?

@mattreid1
Copy link
Owner

I don't have any issue with the code itself, but I struggle to see why it's needed?

As mentioned in #4, app.listen() does not block further code execution and returns the server object. Any code that would be in a callback can just be run after it (see below).

const server = app.listen();
console.log(server.hostname); // http://0.0.0.0:3000/

if (someCondition === true) server.stop();

If I'm missing something let me know!

@fdemir
Copy link
Author

fdemir commented Aug 8, 2022

I don't have any issue with the code itself, but I struggle to see why it's needed?

As mentioned in #4, app.listen() does not block further code execution and returns the server object. Any code that would be in a callback can just be run after it (see below).

const server = app.listen();
console.log(server.hostname); // http://0.0.0.0:3000/

if (someCondition === true) server.stop();

If I'm missing something let me know!

i agree with you definitely. i think this feature should exist as optional because of the developer experience which most of people get used to.

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.

3 participants