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.

Sign up for GitHub

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

Jump to bottom

Automatically using different port to start dev server for non-explicit port #30736

Merged
merged 6 commits into from
Nov 11, 2021
Merged

Automatically using different port to start dev server for non-explicit port #30736

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d62733e
Automatically using different port to start dev server for non-explic…
hyf0 Nov 1, 2021
2327349
return correct port
hyf0 Nov 1, 2021
196dfe5
more reasonable changes
hyf0 Nov 1, 2021
c2ee40a
fix isExplicitPort
hyf0 Nov 1, 2021
3499b85
1. rename isExplicitPort to allowRetry 2.restrict the number of retries
hyf0 Nov 3, 2021
5008aec
Merge branch 'canary' into feat_automatically_re_listen_port
styfle Nov 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/next/cli/next-dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const nextDev: cliCommand = (argv) => {
)
}
}

const allowRetry = !args['--port']
let port: number =
args['--port'] || (process.env.PORT && parseInt(process.env.PORT)) || 3000

Expand All @@ -88,7 +88,11 @@ const nextDev: cliCommand = (argv) => {
// some set-ups that rely on listening on other interfaces
const host = args['--hostname']

startServer({ dir, dev: true, isNextDevCommand: true }, port, host)
startServer(
{ dir, dev: true, isNextDevCommand: true, allowRetry },
port,
host
)
.then(async ({ app, actualPort }) => {
const appUrl = `http://${
!host || host === '0.0.0.0' ? 'localhost' : host
Expand Down
20 changes: 18 additions & 2 deletions packages/next/server/lib/start-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import http from 'http'
import next from '../next'
import { warn } from '../../build/output/log'

export default async function start(
serverOptions: any,
Expand All @@ -18,8 +19,23 @@ export default async function start(
requestHandler = app.getRequestHandler()

await new Promise<void>((resolve, reject) => {
// This code catches EADDRINUSE error if the port is already in use
srv.on('error', reject)
let retryCount = 0
srv.on('error', (err: NodeJS.ErrnoException) => {
// This code catches EADDRINUSE error if the port is already in use
if (
err.code === 'EADDRINUSE' &&
serverOptions.allowRetry &&
port &&
retryCount < 10
) {
warn(`Port ${port} is in use, trying ${port + 1} instead.`)
port += 1
retryCount += 1
srv.listen(port, hostname)
styfle marked this conversation as resolved.
Show resolved Hide resolved
} else {
reject(err)
}
})
Copy link
Member

Choose a reason for hiding this comment

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

Take a look at vercel dev to see how it works. We should probably match the same behavior for next dev too

https://github.com/vercel/vercel/blob/273718e0b78447bbd5b6092df720877099a18752/packages/cli/src/util/dev/server.ts#L845-L869

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for all the guidance.
I'm still new to next codebase, so I try to limit my changes to a minimum level. If we follow vercel dev, does it mean we should also make changes to these codes?

next.js/packages/next/cli/next-dev.ts

Lines 102 to 126 in 5f19f76

.catch((err) => {
if (err.code === 'EADDRINUSE') {
let errorMessage = `Port ${port} is already in use.`
const pkgAppPath = require('next/dist/compiled/find-up').sync(
'package.json',
{
cwd: dir,
}
)
const appPackage = require(pkgAppPath)
if (appPackage.scripts) {
const nextScript = Object.entries(appPackage.scripts).find(
(scriptLine) => scriptLine[1] === 'next'
)
if (nextScript) {
errorMessage += `\nUse \`npm run ${nextScript[0]} -- -p <some other port>\`.`
}
}
console.error(errorMessage)
} else {
console.error(err)
}
process.nextTick(() => process.exit(1))
})
}

srv.on('listening', () => resolve())
srv.listen(port, hostname)
})
Expand Down