-
-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
WIP prompt if port is already in use. #101
Changes from 3 commits
552b988
10c6adf
f0a0f68
2252344
ac6d9d2
aa40a96
ea71a78
63b4290
c1deb6a
c0f391a
245676c
418e62a
78c4bf5
5cf40d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,9 @@ var WebpackDevServer = require('webpack-dev-server'); | |
var config = require('../config/webpack.config.dev'); | ||
var execSync = require('child_process').execSync; | ||
var opn = require('opn'); | ||
var detect = require('detect-port'); | ||
var readline = require('readline'); | ||
var PORT = 3000; | ||
|
||
// TODO: hide this behind a flag and eliminate dead code on eject. | ||
// This shouldn't be exposed to the user. | ||
|
@@ -66,62 +69,89 @@ function clearConsole() { | |
} | ||
|
||
var compiler = webpack(config, handleCompile); | ||
compiler.plugin('invalid', function () { | ||
clearConsole(); | ||
console.log('Compiling...'); | ||
}); | ||
compiler.plugin('done', function (stats) { | ||
clearConsole(); | ||
var hasErrors = stats.hasErrors(); | ||
var hasWarnings = stats.hasWarnings(); | ||
if (!hasErrors && !hasWarnings) { | ||
console.log(chalk.green('Compiled successfully!')); | ||
console.log(); | ||
console.log('The app is running at http://localhost:3000/'); | ||
console.log(); | ||
return; | ||
} | ||
detect(PORT, function(error, _port) { | ||
|
||
var json = stats.toJson(); | ||
var formattedErrors = json.errors.map(message => | ||
'Error in ' + formatMessage(message) | ||
); | ||
var formattedWarnings = json.warnings.map(message => | ||
'Warning in ' + formatMessage(message) | ||
); | ||
if (PORT !== _port) { | ||
var rl = readline.createInterface({ | ||
input: process.stdin, | ||
output: process.stdout | ||
}); | ||
|
||
if (hasErrors) { | ||
console.log(chalk.red('Failed to compile.')); | ||
console.log(); | ||
if (formattedErrors.some(isLikelyASyntaxError)) { | ||
// If there are any syntax errors, show just them. | ||
// This prevents a confusing ESLint parsing error | ||
// preceding a much more useful Babel syntax error. | ||
formattedErrors = formattedErrors.filter(isLikelyASyntaxError); | ||
} | ||
formattedErrors.forEach(message => { | ||
console.log(message); | ||
console.log(); | ||
rl.question('Something is already running at http://localhost:' + PORT + '. Would you like to run the app at another port instead? [Y/n] ', function(answer){ | ||
if(answer === 'Y') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: need space after There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You can see an example of this here. The only difference is that there, |
||
PORT = _port; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this gets reassigned, it isn’t a constant, so it shouldn’t |
||
config.entry = config.entry.map(function(c){ return c.replace(/(\d+)/g, PORT) }); // Replace the port in webpack config | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same nit here |
||
compiler = webpack(config, handleCompile); | ||
setupCompiler(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It’s bad that we repeat this code in this and the other branch below. This makes it easy to change something in one place and forget to change it in the other one. Instead, let’s split the logic into something like function choosePort() {
return new Promise(resolve => {
detect(...)
})
} and then do something like choosePort().then(port => {
setupCompiler(port);
...
}); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually it looks like |
||
runDevServer(); | ||
} | ||
rl.close(); | ||
}); | ||
// If errors exist, ignore warnings. | ||
return; | ||
} else { | ||
runDevServer(); | ||
setupCompiler(); | ||
} | ||
}); | ||
|
||
if (hasWarnings) { | ||
console.log(chalk.yellow('Compiled with warnings.')); | ||
console.log(); | ||
formattedWarnings.forEach(message => { | ||
console.log(message); | ||
function setupCompiler() { | ||
compiler.plugin('invalid', function () { | ||
clearConsole(); | ||
console.log('Compiling...'); | ||
}); | ||
|
||
compiler.plugin('done', function (stats) { | ||
clearConsole(); | ||
var hasErrors = stats.hasErrors(); | ||
var hasWarnings = stats.hasWarnings(); | ||
if (!hasErrors && !hasWarnings) { | ||
console.log(chalk.green('Compiled successfully!')); | ||
console.log(); | ||
}); | ||
console.log('The app is running at http://localhost:' + PORT + '/'); | ||
console.log(); | ||
return; | ||
} | ||
|
||
console.log('You may use special comments to disable some warnings.'); | ||
console.log('Use ' + chalk.yellow('// eslint-disable-next-line') + ' to ignore the next line.'); | ||
console.log('Use ' + chalk.yellow('/* eslint-disable */') + ' to ignore all warnings in a file.'); | ||
} | ||
}); | ||
var json = stats.toJson(); | ||
var formattedErrors = json.errors.map(message => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason we're using arrow functions and standard functions in the same file? I don't mind either syntax but I think consistency is a good idea. That is, unless we need the syntax difference for the different ways they handle things, but if it's just arbitrary I'd prefer we just stuck with one :) |
||
'Error in ' + formatMessage(message) | ||
); | ||
var formattedWarnings = json.warnings.map(message => | ||
'Warning in ' + formatMessage(message) | ||
); | ||
|
||
if (hasErrors) { | ||
console.log(chalk.red('Failed to compile.')); | ||
console.log(); | ||
if (formattedErrors.some(isLikelyASyntaxError)) { | ||
// If there are any syntax errors, show just them. | ||
// This prevents a confusing ESLint parsing error | ||
// preceding a much more useful Babel syntax error. | ||
formattedErrors = formattedErrors.filter(isLikelyASyntaxError); | ||
} | ||
formattedErrors.forEach(message => { | ||
console.log(message); | ||
console.log(); | ||
}); | ||
// If errors exist, ignore warnings. | ||
return; | ||
} | ||
|
||
if (hasWarnings) { | ||
console.log(chalk.yellow('Compiled with warnings.')); | ||
console.log(); | ||
formattedWarnings.forEach(message => { | ||
console.log(message); | ||
console.log(); | ||
}); | ||
|
||
console.log('You may use special comments to disable some warnings.'); | ||
console.log('Use ' + chalk.yellow('// eslint-disable-next-line') + ' to ignore the next line.'); | ||
console.log('Use ' + chalk.yellow('/* eslint-disable */') + ' to ignore all warnings in a file.'); | ||
} | ||
}); | ||
} | ||
|
||
function openBrowser() { | ||
function openBrowser(port) { | ||
if (process.platform === 'darwin') { | ||
try { | ||
// Try our best to reuse existing tab | ||
|
@@ -130,7 +160,7 @@ function openBrowser() { | |
execSync( | ||
'osascript ' + | ||
path.resolve(__dirname, './openChrome.applescript') + | ||
' http://localhost:3000/' | ||
' http://localhost:' + port + '/' | ||
); | ||
return; | ||
} catch (err) { | ||
|
@@ -139,21 +169,23 @@ function openBrowser() { | |
} | ||
// Fallback to opn | ||
// (It will always open new tab) | ||
opn('http://localhost:3000/'); | ||
opn('http://localhost:' + port + '/'); | ||
} | ||
|
||
new WebpackDevServer(compiler, { | ||
historyApiFallback: true, | ||
hot: true, // Note: only CSS is currently hot reloaded | ||
publicPath: config.output.publicPath, | ||
quiet: true | ||
}).listen(3000, function (err, result) { | ||
if (err) { | ||
return console.log(err); | ||
} | ||
function runDevServer() { | ||
new WebpackDevServer(compiler, { | ||
historyApiFallback: true, | ||
hot: true, // Note: only CSS is currently hot reloaded | ||
publicPath: config.output.publicPath, | ||
quiet: true | ||
}).listen(PORT, 'localhost', function (err, result) { | ||
if (err) { | ||
return console.log(err); | ||
} | ||
|
||
clearConsole(); | ||
console.log(chalk.cyan('Starting the development server...')); | ||
console.log(); | ||
openBrowser(); | ||
}); | ||
clearConsole(); | ||
console.log(chalk.cyan('Starting the development server...')); | ||
console.log(); | ||
openBrowser(PORT); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nit, can you add a space between
)
and{
:function(answer) {