-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[WIP]: add "open" option to node api to allow opening in browser #1627
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
Changes from all commits
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 |
---|---|---|
|
@@ -27,6 +27,9 @@ const killable = require('killable'); | |
const del = require('del'); | ||
const chokidar = require('chokidar'); | ||
|
||
const open = require('opn'); | ||
const createDomain = require('./utils/createDomain'); | ||
|
||
const express = require('express'); | ||
|
||
const compress = require('compression'); | ||
|
@@ -77,6 +80,22 @@ const STATS = { | |
errorDetails: false | ||
}; | ||
|
||
function openBrowser(uri, options, log) { | ||
let openOptions = {}; | ||
let openMessage = 'Unable to open browser'; | ||
|
||
if (typeof options.open === 'string') { | ||
openOptions = { app: options.open }; | ||
openMessage += `: ${options.open}`; | ||
} | ||
|
||
open(uri + (options.openPage || ''), openOptions).catch(() => { | ||
log.warn( | ||
`${openMessage}. If you are running in a headless environment, please do not use the --open flag` | ||
); | ||
}); | ||
} | ||
|
||
function Server (compiler, options = {}, _log) { | ||
this.log = _log || createLogger(options); | ||
|
||
|
@@ -622,6 +641,8 @@ function Server (compiler, options = {}, _log) { | |
this.listeningApp = http.createServer(app); | ||
} | ||
|
||
this.options = options; | ||
|
||
killable(this.listeningApp); | ||
|
||
// Proxy websockets without the initial http request | ||
|
@@ -804,6 +825,12 @@ Server.prototype.listen = function (port, hostname, fn) { | |
if (fn) { | ||
fn.call(this.listeningApp, err); | ||
} | ||
|
||
if (this.options.open) { | ||
const suffix = (this.options.inline !== false || this.options.lazy === true ? '/' : '/webpack-dev-server/'); | ||
const uri = createDomain({ host: hostname, port }, this.listeningApp) + suffix; | ||
openBrowser(uri, this.options, this.log); | ||
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 is should be in listen event and remove logic for open from bin file 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.
I'm not sure what do you mean by this. The previous comment says that open logic should be kept in bin 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. Also socksjs doesn't seem to register a 'listen' event. On what object should i listen for and where should i call it? 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. 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. But if I listen with express what port should i listen on? Also i don't understand the reason for listening on another port with express. Can you please explain this. |
||
} | ||
}); | ||
|
||
return returnValue; | ||
|
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.
Don't move this from utils