Use next available port #69
Conversation
| @@ -14,11 +15,12 @@ export type Inject = { | |||
|
|
|||
| async function actualRun(appInfo: AppInfo, options: Options, inject: Inject): Promise<void> { | |||
| const { injectFrontend, injectSim, getIndexHtml } = inject; | |||
| options.basePort = await detectPort(options.basePort); | |||
There was a problem hiding this comment.
Before this line options.basePort is either 7000 (the default) or the value provided by --port and detectPort() then tries that port or finds the next available in sequence.
| this.currentPort++; | ||
| const port = this.currentPort; | ||
| async add(): Promise<Instance> { | ||
| const port = await detectPort(this.currentPort + 1); |
There was a problem hiding this comment.
This works quite nicely actually, so if you start e.g. webxdc-dev run /path/to/some.xdc in one console (would pick port 7000 and also have port 7001 and 7002 for the instances) and then do webxdc-dev run --port 7002 /path/to/some.xdc then the backend would start at 7003 (since 7002 was already taken by one of the instances in the first window) with two instances at 7004 and 7005.
If you then click Add Instance in the first window, then the next instance will get port 7006:
| @@ -18,4 +18,4 @@ const program = createProgram({ | |||
| }, | |||
| }); | |||
|
|
|||
| program.parse(); | |||
| program.parseAsync(); | |||
There was a problem hiding this comment.
Need to call .parseAsync() if the action callback is async.
| this.currentPort++; | ||
| const port = this.currentPort; | ||
| async add(): Promise<Instance> { | ||
| const port = await detectPort(this.currentPort + 1); | ||
| if (this.instances.has(port)) { | ||
| throw new Error(`Already have Webxdc instance at port: ${port}`); | ||
| } |
There was a problem hiding this comment.
I think this if-statement is redundant now since detectPort() will return a valid port that's not taken. But I'm fine with leaving it here for good measure.
|
@Simon-Laux Mind having a quick look at this? |
| @@ -83,6 +83,7 @@ | |||
| "adm-zip": "^0.5.9", | |||
| "body-parser": "^1.20.0", | |||
| "commander": "^9.3.0", | |||
| "detect-port": "^2.1.0", | |||
There was a problem hiding this comment.
maybe dumb question: couldn't you just try to listen on the port and catch the error if the address is already in use? instead of adding a new dependency.
There was a problem hiding this comment.
It's not a dumb question. That's basically what the dependency does.
There was a problem hiding this comment.
I can swap the dependency out for a simpler version of this for sure, no problem. I just wanted to solve the problem first 😄
I'm actually not very familiar with this codebase, so from my perspective everything is fine as long as it works in the end. |
Closes #68