-
-
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
[WIP]: add "open" option to node api to allow opening in browser #1627
Conversation
Does this conflict with #1540? |
Codecov Report
@@ Coverage Diff @@
## master #1627 +/- ##
==========================================
- Coverage 74.27% 74.02% -0.25%
==========================================
Files 10 10
Lines 688 693 +5
==========================================
+ Hits 511 513 +2
- Misses 177 180 +3
Continue to review full report at Codecov.
|
`${openMessage}. If you are running in a headless environment, please do not use the --open flag` | ||
); | ||
}); | ||
} |
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
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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
remove logic for open from bin file
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 comment
The 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 comment
The 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 comment
The 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.
@evilebottnawi @michael-ciniawsky it seems like #1540 does the same thing as this PR. Should this PR be closed in favor of that PR? |
@amilajack PR was abadoned, so you can continue develop |
Codecov Report
@@ Coverage Diff @@
## master #1627 +/- ##
==========================================
- Coverage 74.27% 74.02% -0.25%
==========================================
Files 10 10
Lines 688 693 +5
==========================================
+ Hits 511 513 +2
- Misses 177 180 +3
Continue to review full report at Codecov.
|
friendly ping @amilajack |
For Bugs and Features; did you add new tests?
Motivation / Use-Case
Node api does not support open option
Breaking Changes
Additional Info