preactjs / preact-cli

😺 Your next Preact PWA starts in 30 seconds.
MIT License
4.69k stars 375 forks source link

Suggest preact commands to run dev server on new port if 8080 is already being used #179

Closed reznord closed 7 years ago

reznord commented 7 years ago

As of now, when we run a dev server using preact serve or preact watch by default opens on 8080 port.

If there is a process which runs on 8080 already, preact doesn't ask the user whether he wants to run on a new port. As of now it terminates the process.

It would be great if we can suggest the CLI to ask the user whether he wants to run on a different port.

thangngoc89 commented 7 years ago

This would be a great addition. I have some code snippets I can paste here if you need it

reznord commented 7 years ago

I have some code snippets I can paste here if you need it

Cool, I'd love it

thangngoc89 commented 7 years ago

this time, it's 3 lines

const getPort = require("get-port")

getPort(3000).then(port => {
    console.log(port);
    // Will use 3000 if available, otherwise fall back to a random port
});

if 3000 !== port, then ask user if they want to spawn in other port.

thangngoc89 commented 7 years ago

on *nix, you can use this command this see which process is binding to a node, but it may require sudo

image

rkostrzewski commented 7 years ago

We can always simply spawn http server using 0 port. Even if not all servers accept port 0 we could spawn http server at port 0 get actual port & kill it.

thangngoc89 commented 7 years ago

@rkostrzewski that's what get-port package doing.

'use strict';
const net = require('net');

const getPort = port => new Promise((resolve, reject) => {
    const server = net.createServer();

    server.unref();
    server.on('error', reject);

    server.listen(port, () => {
        const port = server.address().port;
        server.close(() => {
            resolve(port);
        });
    });
});

module.exports = preferredPort => preferredPort ?
    getPort(preferredPort).catch(() => getPort(0)) :
    getPort(0);
reznord commented 7 years ago

CRA does this in a pretty nice way !! I would just take the handling of PORT from CRA and use it here !! That is fine right?

developit commented 7 years ago

We can use the popular get-port module to handle this:

getPort(3000).then(port => {
    console.log(port);
    // Will use 3000 if available, otherwise fall back to a random port 
});
rkostrzewski commented 7 years ago

@thangngoc89 I didn't see that require call 😆

thangngoc89 commented 7 years ago

@developit I talked about this and no one listen https://github.com/developit/preact-cli/issues/179#issuecomment-312562589

reznord commented 7 years ago

okay, so get-port it is !! I'll do it right away !!

developit commented 7 years ago

Whoops, forgot to read the whole thread @thangngoc89 - sorry about that! I like that get-port lets us specify a preferred port before grabbing a random one: that helps with documentation because we can say "then open http://localhost:3000" and newcomers will not have to look at the output to find their generated port.

lukeed commented 7 years ago

I think we should always be running a random port (if 8080 is taken). If a user passes --port and its in use, we should stop & exit with a message why.

thangngoc89 commented 7 years ago

@lukeed I saw a discussions about this between @-gaearon and others. From UX pov, we should not spawn in random port (if 8080 is taken). Because users might spawning it in other terminal tab. instead, ask user what to do

lukeed commented 7 years ago

Okay 👍 Just reciting from what I've seen & have liked personally. Sounds good either way

reznord commented 7 years ago

get-port doesn't suggest a free port. We can do that with detect-port.

Any thoughts?

thangngoc89 commented 7 years ago

@reznord What do you mean by free port? get-port return a new port in case preferred port was occupied

reznord commented 7 years ago

get-port return a new port in case preferred port was occupied

Oh, I didn't know this. Then we can go with get-port itself.