Use port manager #936
Use port manager #936
Conversation
| @@ -61,6 +70,7 @@ class DevServerManager { | |||
| this.debug = debug; | |||
| this.componentsByType = this.arrangeComponentsByType(components); | |||
|
|
|||
| await startPortManagerServer(); | |||
There was a problem hiding this comment.
My comment here is relevant. It should enable you to catch port conflicts when attempting to start up the server manager and log a useful error here
| @@ -86,6 +93,7 @@ class DevServerManager { | |||
| debug: this.debug, | |||
| httpClient, | |||
| projectConfig, | |||
| requestPorts, | |||
There was a problem hiding this comment.
Not a blocker - I wonder if this is something that we should be passing down to the servers or if they should just import it directly from local-dev-lib. They will likely already have local-dev-lib as a dep anyways.
There was a problem hiding this comment.
Yeah I thought about this a bit. I ended up going with this approach because the CLI is responsible for starting the port manager server. So when the server gets it from the DevServerManager, it comes with a guarantee that the port manager is actually running. In practice though I don't think it would really make a difference, so not opposed to changing this.
packages/cli/lib/DevServerManager.js
Outdated
| @@ -113,6 +121,10 @@ class DevServerManager { | |||
| await serverInterface.cleanup(); | |||
| } | |||
| }); | |||
|
|
|||
| if (!portManagerHasActiveServers()) { | |||
There was a problem hiding this comment.
portManagerHasActiveServers is async so you'll need await here. The stop function is also async and we might want to wait for that so we can handle errors if there are any
Description and Context
See https://git.hubteam.com/HubSpot/hubspot-cli-issues/issues/462
This sets up the DevServerManager in the CLI to fetch ports from the port manager for each dev server it needs to run. It then passes those ports to the respective dev servers (for now, just the UIE dev server). This is set up to only allow one
hs project devprocess but in the future, we could support more than one by appending component or project ids to server instance ids.Related port manager PRs:
HubSpot/hubspot-local-dev-lib#42
https://git.hubteam.com/HubSpot/ui-extensibility/pull/1566
Todo
This PR adds a dependency on local-dev-lib, so will need to publish its first (non-prerelease) release. Local dev lib also currently uses a newer node version, so we'll either have to change that dep or see if we can upgrade node on the CLI
Who to Notify
@brandenrodgers @kemmerle