Skip to content

Add dynamic port allocation #913

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

Merged
merged 5 commits into from
Sep 30, 2017
Merged

Add dynamic port allocation #913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f912a59
Add dynamic port allocation
adi518 Sep 8, 2017
be9f547
Add error handling to port allocation
adi518 Sep 8, 2017
f538e19
Remove forgotten listen statement from dev-server.js
adi518 Sep 8, 2017
bce2fdd
Set dynamically allocated port to environment variable `PORT`
adi518 Sep 9, 2017
dac5a0b
Merge branch 'develop' into develop
LinusBorg Sep 30, 2017
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
32 changes: 22 additions & 10 deletions template/build/dev-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,35 @@ app.use(staticPath, express.static('./static'))

const uri = 'http://localhost:' + port

let _resolve
const readyPromise = new Promise(resolve => {
var _resolve
var _reject
var readyPromise = new Promise((resolve, reject) => {
_resolve = resolve
_reject = reject
})

var server
var portfinder = require('portfinder')
portfinder.basePort = port

console.log('> Starting dev server...')
devMiddleware.waitUntilValid(() => {
console.log('> Listening at ' + uri + '\n')
// when env is testing, don't need open it
if (autoOpenBrowser && process.env.NODE_ENV !== 'testing') {
opn(uri)
}
_resolve()
portfinder.getPort((err, port) => {
if (err) {
_reject(err)
}
process.env.PORT = port
var uri = 'http://localhost:' + port
console.log('> Listening at ' + uri + '\n')
// when env is testing, don't need open it
if (autoOpenBrowser && process.env.NODE_ENV !== 'testing') {
Copy link
Contributor

@LinusBorg LinusBorg Sep 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

When we talked about adding a check for 'testing' environment, I didn't think about skipping the opn(uri) - I was thinking about skipping the whole portfinder logic.

The point is: for e2e tests, we currently try to access the dev-server through the port that is defined in the config (see this line). If the portfinder starts the def-server at a different port because that port is in use, e2e tests will connect to that wrong port anyway.

So either we find a way to tell the e2e test runner about the new found port, or we have to skip portfinder completely hen running e2e tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought we already did that. That is, updating the environment variable read by the e2e config.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, right - sorry, I missed that.

opn(uri)
}
server = app.listen(port)
_resolve()
})
})

const server = app.listen(port)

module.exports = {
ready: readyPromise,
close: () => {
Expand Down
1 change: 1 addition & 0 deletions template/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
"vue-loader": "^13.0.4",
"vue-style-loader": "^3.0.1",
"vue-template-compiler": "^2.4.2",
"portfinder": "^1.0.13"
"webpack": "^3.5.5",
"webpack-dev-middleware": "^1.12.0",
"webpack-hot-middleware": "^2.18.2",
Expand Down