Dynamic port switching #516
Conversation
lib/start-server.js
Outdated
| // Try again but with port + 1 | ||
| port += 1; | ||
| checkPort(); | ||
| // process.exit(1); |
There was a problem hiding this comment.
This line should be entirely removed.
There was a problem hiding this comment.
Hm, do you mean that those 4 lines should all be removed?
There was a problem hiding this comment.
No, just the line you commented out. There's no need to leave it in
|
@JoelMarcey Have a look at this PR when you get the chance! It's really useful and the code looks pretty good! 😄 |
|
@InternetExplorer7 This looks good. Thanks! Can you just remove the comment that @yangshun mentioned and then I can approve and merge this? |
lib/start-server.js
Outdated
| port += 1; | ||
| numAttempts += 1; | ||
| checkPort(); | ||
| } else if (inUse && numAttempts >= maxAttempts) { |
There was a problem hiding this comment.
I don't think this will ever get executed since the first inUse check above will override it. I think you want the numAttempts check within the if (inUse) check above.
There was a problem hiding this comment.
Oops, this is embarrassing. One moment and I'll fix this.
| @@ -14,7 +14,7 @@ | |||
| "url": "https://github.com/facebook/Docusaurus.git" | |||
| }, | |||
| "scripts": { | |||
| "ci-check": "yarn prettier:diff", | |||
| "ci-check": "yarn prettier && yarn prettier:diff", | |||
There was a problem hiding this comment.
@InternetExplorer7 This should not be added here. yarn ci-check is run in Circle this change makes the code prettified before diffing, so it is guaranteed to pass.
The check for whether the code has been prettified is essentially rendered ineffective by this change as even if submitted code has been prettified, this command will fix it and pass the CI but the changes are not committed to the branch.
I'll submit a PR to revert this line. cc @JoelMarcey
|
Ah - I don't even think I noticed that. Nice catch @yangshun 👍 |
| throw ex; | ||
| }, 0); | ||
| }); | ||
| var port = process.env.PORT || 3000; |
There was a problem hiding this comment.
This changes the behavior from listening to the --port argument on the command line to use the PORT environment variable.
There was a problem hiding this comment.
I'll revert this line.
EDIT: Just saw #588
The behavior was changed in #516. With this change, both the command line argument and environment variable will work.
The behavior was changed in #516. With this change, both the command line argument and environment variable will work.
Motivation
This PR includes a patch to resolve this feature request when default port 3000 is already in use by another program: #353
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Tested by re-running server to confirm page is still rendered properly and that the page is still being correctly served. I then ran 'yarn test' and that passed all test cases.
I also tested by reserving default port 3000 to see if the server would automatically change ports without exiting. It did:
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)
Change Impact Analysis
For #353, this was a server-side feature implementation that automatically changes the port (adds 1 to the port) until an available port is found. Previously, if the default port was not found then the app would simply exit. Also, if an environmental variable for Port is found, then that port shall be used. If that port is also not available, then it will continue attempting to find another open port. Changing the port does not effect the rest of the components significantly. Previously, a used port would exit the entire process making the server unusable until either A) the port was opened by the user or B) the port number was changed in the server code.
Do any existing requirements in the baseline conflict with the proposed change?
No.
Do any other pending requirements changes conflict with the proposed change?
No.
What are the business or technical consequences of not making a change?
Previously, if port 3000 was already in-use, then this app would simply exit and the pages would not render at all.
Will the proposed change adversely affect performance requirements or other quality attributes?
Yes, if the port is changed so that the server then runs, then the performance will be improved since the app's now in a running state.
Is the proposed change feasible within known technical constraints and current staff skills?
Yes.
Will the proposed change place unacceptable demands on any computer resources required for the development, test, or operating environments?
Some (albeit immeasurable) performance decrease may occur for finding the next available port, especially if a large range of ports after 3000 are also already in use. e.g. (3000), (3000 + 1), ..., (3000 + n).
Must any tools be acquired to implement and test the change?
Contributors may use the built-in test suite to test the change using command yarn test. They may also go ahead and try accessing seeing if the server is even up.
How will the proposed change affect the sequence, dependencies, effort, or duration of any tasks currently in the project plan?
No dependencies needed to be changed and no new dependencies need to be added in order to implement this change.
Will prototyping or other use input be required to verify the proposed change?
Yes, if navigating to localhost: where port is the port that was found returns the homepage, then you know that it worked.
How much effort that has already been invested in the project will be lost if this change is accepted?
Possibly a few hours.
Will the proposed change cause an increase in product unit cost, such as by increasing third-party product licensing fees?
No.
Will the change affect any marketing, manufacturing, training, or customer support plans?
No.
Design Decisions
For #353, on top of writing an algorithm for finding a new, available port, I also give the user an option to define their own port by using environmental variables. If an environmental variable does not exist, then I proceed with default port 3000 and run the port checking algorithm each time until an open port is found.