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

feat: Port in use checking for development server #6437

Merged
merged 22 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d0ed643
Initial port in use check implementation
Josh-Walker-GM Sep 21, 2022
0bc39a7
Added suggested port, initial prompt and ensured consistent logging s…
Josh-Walker-GM Sep 22, 2022
ca0b549
Removed excess style import
Josh-Walker-GM Sep 22, 2022
e47928c
Changed to use node-portfinder, tidied console logging and added init…
Josh-Walker-GM Sep 22, 2022
466cecd
fix(deps): pin portfinder and dedupe yarn.lock
jtoar Oct 1, 2022
d0e8950
Improved conditionals
Josh-Walker-GM Oct 1, 2022
8b0488f
Added port flag to the watch api-server
Josh-Walker-GM Oct 2, 2022
d33584c
Added working port checking
Josh-Walker-GM Oct 2, 2022
d1aba20
Merge remote-tracking branch 'upstream/main' into dev-server-port
Josh-Walker-GM Oct 12, 2022
95044cd
Made ports consistently a number type, fixed typo in port use warning…
Josh-Walker-GM Oct 12, 2022
90baa43
Added default port options to mock config and updated inline snapshot…
Josh-Walker-GM Oct 12, 2022
3d06f93
Apply suggestions from code review
Josh-Walker-GM Nov 6, 2022
b1ba9bc
Applying more suggestions from code review
Josh-Walker-GM Nov 6, 2022
94bf0be
Changed to a simple error warning
Josh-Walker-GM Nov 9, 2022
0b15df5
style: remove "=" after flag
jtoar Nov 16, 2022
95938da
fix: substring test and snapshot
jtoar Nov 16, 2022
7652b2c
Apply suggestions from code review
Josh-Walker-GM Nov 16, 2022
a6990a5
Remove unused function
Josh-Walker-GM Nov 16, 2022
7c8676c
Merge branch 'main' into dev-server-port
jtoar Nov 17, 2022
bdb32e5
Merge remote-tracking branch 'upstream/main' into dev-server-port
Josh-Walker-GM Nov 18, 2022
b2aaea0
Merge branch 'main' into dev-server-port
jtoar Nov 21, 2022
b6b0aa6
Merge branch 'main' into dev-server-port
jtoar Nov 21, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/api-server/src/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ const argv = yargs(hideBin(process.argv))
description: 'Debugging port',
type: 'number',
})
.option('port', {
alias: 'p',
description: 'Port',
type: 'number',
})
.help()
.alias('help', 'h')
.parseSync()
Expand Down Expand Up @@ -75,10 +80,15 @@ const rebuildApiServer = () => {
forkOpts.execArgv = forkOpts.execArgv.concat([`--inspect=${debugPort}`])
}

let port = argv['port']
if (!port) {
port = getConfig().api.port
}

// Start API server
httpServerProcess = fork(
path.join(__dirname, 'index.js'),
['api', '--port', getConfig().api.port.toString()],
['api', '--port', port.toString()],
forkOpts
)
} catch (e) {
Expand Down
1 change: 1 addition & 0 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"param-case": "3.0.4",
"pascalcase": "1.0.0",
"pluralize": "8.0.0",
"portfinder": "1.0.32",
"prettier": "2.7.1",
"prisma": "4.3.1",
"prompts": "2.4.2",
Expand Down
17 changes: 13 additions & 4 deletions packages/cli/src/commands/__tests__/dev.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,11 @@ describe('yarn rw dev', () => {

it('Should run api and web dev servers, and by default', async () => {
getConfig.mockReturnValue({
web: {},
web: {
port: 8910,
},
api: {
port: 8911,
debugPort: 18911,
},
})
Expand All @@ -92,16 +95,19 @@ describe('yarn rw dev', () => {
)

expect(apiCommand.command).toMatchInlineSnapshot(
`"yarn cross-env NODE_ENV=development NODE_OPTIONS=--enable-source-maps yarn nodemon --quiet --watch "/mocked/project/redwood.toml" --exec "yarn rw-api-server-watch --debug-port 18911 | rw-log-formatter""`
`"yarn cross-env NODE_ENV=development NODE_OPTIONS=--enable-source-maps yarn nodemon --quiet --watch "/mocked/project/redwood.toml" --exec "yarn rw-api-server-watch --port=8911 --debug-port 18911 | rw-log-formatter""`
)

expect(generateCommand.command).toEqual('yarn rw-gen-watch')
})

it('Debug port passed in command line overrides TOML', async () => {
getConfig.mockReturnValue({
web: {},
web: {
port: 8910,
},
api: {
port: 8911,
debugPort: 505050,
},
})
Expand All @@ -121,8 +127,11 @@ describe('yarn rw dev', () => {

it('Can disable debugger by setting toml to false', async () => {
getConfig.mockReturnValue({
web: {},
web: {
port: 8910,
},
api: {
port: 8911,
debugPort: false,
},
})
Expand Down
119 changes: 116 additions & 3 deletions packages/cli/src/commands/devHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import fs from 'fs'
import { argv } from 'process'

import concurrently from 'concurrently'
import portfinder from 'portfinder'
import prompts from 'prompts'

import { getConfig } from '@redwoodjs/internal/dist/config'
import { shutdownPort } from '@redwoodjs/internal/dist/dev'
Expand All @@ -14,6 +16,26 @@ import { generatePrismaClient } from '../lib/generatePrismaClient'

const defaultApiDebugPort = 18911

/**
* Finds a free port
* @param {[Number]} requestedPort Port to start searching from
* @param {[Number[]]} excludePorts Array of port numbers to exclude
* @return {[Number]} A free port equal or higher than requestedPort but not within excludePorts. If no port can be found then returns -1
*/
async function getFreePort(requestedPort, excludePorts = []) {
try {
let freePort = await portfinder.getPortPromise({
port: requestedPort,
})
if (excludePorts.includes(freePort)) {
freePort = await getFreePort(freePort + 1, excludePorts)
}
return freePort
} catch (error) {
return -1
}
}

export const handler = async ({
side = ['api', 'web'],
forward = '',
Expand All @@ -23,6 +45,97 @@ export const handler = async ({
}) => {
const rwjsPaths = getPaths()

let apiPort = getConfig().api.port
let webPort = getConfig().web.port

// Check api port
if (side.includes('api')) {
const freePort = await getFreePort(apiPort)
if (freePort === -1) {
console.error(
c.error(
`Requested API port of ${apiPort} is already in use and no neighbouring port is available! Cannot start development server.`
)
)
process.exit(1)
}
if (freePort !== apiPort) {
console.log(
c.warning(
`Requested API port of ${apiPort} is already in use however ${freePort} is available.`
)
)
const useAvailablePort = await prompts({
type: 'confirm',
name: 'port',
message: `Do you wish to use port ${freePort} instead?`,
initial: true,
active: 'Yes',
inactive: 'No',
})
if (!useAvailablePort.port) {
console.log(c.info(`The API port can be updated in 'redwood.toml'.`))
process.exit(0)
}
}
apiPort = freePort

// TODO: Check the apiDebugPort too?
}

// Check web port
if (side.includes('web')) {
// Check for specific forwarded web port

const forwardedPortMatches = forward.match(
/--port=[0-9][0-9]?[0-9]?[0-9]?[0-9]? ?/
)
const forwardedPortSet =
forwardedPortMatches && forwardedPortMatches.length == 1
if (forwardedPortSet) {
webPort = forwardedPortMatches[0]
.substring(forwardedPortMatches[0].indexOf('=') + 1)
.trim()
}

const freePort = await getFreePort(webPort, [apiPort])
if (freePort === -1) {
console.error(
c.error(
`Requested web port of ${webPort} is already in use and no neighbouring port is available! Cannot start development server.`
)
)
process.exit(1)
}
if (freePort !== webPort) {
console.log(
c.warning(
`Requested web port of ${webPort} is already in use however ${freePort} is available.`
)
)
const useAvailablePort = await prompts({
type: 'confirm',
name: 'port',
message: `Do you wish to use port ${freePort} instead?`,
initial: true,
active: 'Yes',
inactive: 'No',
})
if (!useAvailablePort.port) {
console.log(
c.info(
`The web port can be updated in 'redwood.toml' or can be forwarded via the command line parameter like so 'yarn rw dev --fwd="--port=12345"'.`
)
)
process.exit(0)
}
}
webPort = freePort
forward = forwardedPortSet
? forward.replace(forwardedPortMatches[0], ` --port=${freePort}`)
: forward.concat(` --port=${freePort}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since webpack uses the last port, we can just concat it on if it's different. (I also moved this up a block like apiPort = freePort)

Suggested change
}
webPort = freePort
forward = forwardedPortSet
? forward.replace(forwardedPortMatches[0], ` --port=${freePort}`)
: forward.concat(` --port=${freePort}`)
webPort = freePort
forward = forward.concat(` --port=${freePort}`)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with this too. I added a quick comment just to let anyone know the expected behaviour with multiple ports in the forward string. I also added a little check so we only concat on an additional port if it's different from the one they originally specified (via config or forward).

// If needed add the newly chosen port to the webpack forward string. Note: webpack will use the final --port option if more than one are supplied
if (webPort !== freePort) {
  forward = forward.concat(` --port=${freePort}`)
}
webPort = freePort

Hope this is fine?

}

if (side.includes('api')) {
try {
await generatePrismaClient({
Expand All @@ -39,7 +152,7 @@ export const handler = async ({
}

try {
await shutdownPort(getConfig().api.port)
await shutdownPort(apiPort)
} catch (e) {
errorTelemetry(process.argv, `Error shutting down "api": ${e.message}`)
console.error(
Expand All @@ -50,7 +163,7 @@ export const handler = async ({

if (side.includes('web')) {
try {
await shutdownPort(getConfig().web.port)
await shutdownPort(webPort)
} catch (e) {
errorTelemetry(process.argv, `Error shutting down "web": ${e.message}`)
console.error(
Expand Down Expand Up @@ -86,7 +199,7 @@ export const handler = async ({
const jobs = {
api: {
name: 'api',
command: `yarn cross-env NODE_ENV=development NODE_OPTIONS=--enable-source-maps yarn nodemon --quiet --watch "${redwoodConfigPath}" --exec "yarn rw-api-server-watch ${getApiDebugFlag()} | rw-log-formatter"`,
command: `yarn cross-env NODE_ENV=development NODE_OPTIONS=--enable-source-maps yarn nodemon --quiet --watch "${redwoodConfigPath}" --exec "yarn rw-api-server-watch --port=${apiPort} ${getApiDebugFlag()} | rw-log-formatter"`,
prefixColor: 'cyan',
runWhen: () => fs.existsSync(rwjsPaths.api.src),
},
Expand Down
1 change: 1 addition & 0 deletions packages/internal/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"@types/babel__core": "7.1.19",
"@types/findup-sync": "4.0.2",
"@types/fs-extra": "9.0.13",
"@types/node": "16.11.47",
"@types/rimraf": "3.0.2",
"babel-plugin-tester": "10.1.0",
"graphql-tag": "2.12.6",
Expand Down
41 changes: 35 additions & 6 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6619,6 +6619,7 @@ __metadata:
param-case: 3.0.4
pascalcase: 1.0.0
pluralize: 8.0.0
portfinder: 1.0.32
prettier: 2.7.1
prisma: 4.3.1
prompts: 2.4.2
Expand Down Expand Up @@ -6864,6 +6865,7 @@ __metadata:
"@types/babel__core": 7.1.19
"@types/findup-sync": 4.0.2
"@types/fs-extra": 9.0.13
"@types/node": 16.11.47
"@types/rimraf": 3.0.2
babel-plugin-graphql-tag: 3.3.0
babel-plugin-polyfill-corejs3: 0.6.0
Expand Down Expand Up @@ -9360,6 +9362,13 @@ __metadata:
languageName: node
linkType: hard

"@types/node@npm:16.11.47":
version: 16.11.47
resolution: "@types/node@npm:16.11.47"
checksum: a10564a9ebf4568a6efc57ad2b7aa5728c934e798f143e03ed5f3dc05efe1885cc5ca3b94e500912e9544ee92294500bc2360e7a03ab25e78907304aed9b84b1
languageName: node
linkType: hard

"@types/node@npm:16.11.65, @types/node@npm:^14.0.10 || ^16.0.0, @types/node@npm:^14.14.20 || ^16.0.0":
version: 16.11.65
resolution: "@types/node@npm:16.11.65"
Expand Down Expand Up @@ -11256,6 +11265,15 @@ __metadata:
languageName: node
linkType: hard

"async@npm:^2.6.4":
version: 2.6.4
resolution: "async@npm:2.6.4"
dependencies:
lodash: ^4.17.14
checksum: 0ebb3273ef96513389520adc88e0d3c45e523d03653cc9b66f5c46f4239444294899bfd13d2b569e7dbfde7da2235c35cf5fd3ece9524f935d41bbe4efccdad0
languageName: node
linkType: hard

"async@npm:^3.1.0, async@npm:^3.2.0, async@npm:^3.2.3":
version: 3.2.4
resolution: "async@npm:3.2.4"
Expand Down Expand Up @@ -22163,7 +22181,7 @@ __metadata:
languageName: node
linkType: hard

"lodash@npm:4.17.21, lodash@npm:>=4.17.21, lodash@npm:^4.0.0, lodash@npm:^4.11.2, lodash@npm:^4.17.15, lodash@npm:^4.17.19, lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:~4.17.0":
"lodash@npm:4.17.21, lodash@npm:>=4.17.21, lodash@npm:^4.0.0, lodash@npm:^4.11.2, lodash@npm:^4.17.14, lodash@npm:^4.17.15, lodash@npm:^4.17.19, lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:~4.17.0":
version: 4.17.21
resolution: "lodash@npm:4.17.21"
checksum: d8cbea072bb08655bb4c989da418994b073a608dffa608b09ac04b43a791b12aeae7cd7ad919aa4c925f33b48490b5cfe6c1f71d827956071dae2e7bb3a6b74c
Expand Down Expand Up @@ -23082,14 +23100,14 @@ __metadata:
languageName: node
linkType: hard

"mkdirp@npm:^0.5.1, mkdirp@npm:^0.5.3":
version: 0.5.5
resolution: "mkdirp@npm:0.5.5"
"mkdirp@npm:^0.5.1, mkdirp@npm:^0.5.3, mkdirp@npm:^0.5.6":
version: 0.5.6
resolution: "mkdirp@npm:0.5.6"
dependencies:
minimist: ^1.2.5
minimist: ^1.2.6
bin:
mkdirp: bin/cmd.js
checksum: 4469faeeba703bc46b7cdbe3097d6373747a581eb8b556ce41c8fd25a826eb3254466c6522ba823c2edb0b6f0da7beb91cf71f040bc4e361534a3e67f0994bd0
checksum: e2e2be789218807b58abced04e7b49851d9e46e88a2f9539242cc8a92c9b5c3a0b9bab360bd3014e02a140fc4fbc58e31176c408b493f8a2a6f4986bd7527b01
languageName: node
linkType: hard

Expand Down Expand Up @@ -25145,6 +25163,17 @@ __metadata:
languageName: node
linkType: hard

"portfinder@npm:1.0.32":
version: 1.0.32
resolution: "portfinder@npm:1.0.32"
dependencies:
async: ^2.6.4
debug: ^3.2.7
mkdirp: ^0.5.6
checksum: cef8b567b78aabccc59fe8e103bac8b394bb45a6a69be626608f099f454124c775aaf47b274c006332c07ab3f501cde55e49aaeb9d49d78d90362d776a565cbf
languageName: node
linkType: hard

"posix-character-classes@npm:^0.1.0":
version: 0.1.1
resolution: "posix-character-classes@npm:0.1.1"
Expand Down