Skip to content
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

quick fix for HTTPS port from env not working properly by allowing env HMR_PORT alongside env PORT #9715

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from

Conversation

700software
Copy link

↪️ Pull Request

quick fix for #9345

However, I do not like the fact that normalizeOptions is a) responsible for invoking getPort as a fallback & b) unaware of env.PORT.

This will at least solve the immediate issue #9345 and maybe a better refactor of the sequence of these checks can be done later.

💻 Examples

Steps to repro: (from v2 branch)

run two parcel projects, both with PORT= and HMR_PORT= in the .env *and the --https option (no --port option)
the two parcel projects you start up should have different non-conflicting port numbers in their .env

You will notice that the 2nd one givs an error about port 1234 being taken despite the fact that you have PORT specified in env. The reason for this is the first project, while it respects PORT for the https server, is silently running and HMR server on port 1234.

So what you will find is the 2nd parcel project you start up is picking a port number randomly with getPort.

However, if you use this PR, you will see that you can run multiple parcel projects with no port conflicts by relying only on .env configuration and no need to set --port (which in our case we are trying to avoid)

🚨 Test instructions

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant