-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fix HMR server when WebSocket proxy is running on the same port #9432
base: v2
Are you sure you want to change the base?
Conversation
Why the strong emphasis 😄 ? Any particular cases where you're not sure whether they work? Would it be feasible to add an integration test (that your own WS connections can be proxied)?
|
In case you didn't notice, 27 existing tests are also failing. e.g.
|
@mischnic - I added the strong emphasis because I committed without testing, and as a programmer, I'm sure you know that nothing ever works the first time. Haha! Anyway, could you give me the 30-second crash course into running the test suite? I should have some time this week to get this PR working. |
esbuild and Flow also fail, so you need these commands to see all the CI failures locally: yarn lint
yarn flow
yarn test:integration # run all tests
yarn test:integration test/hmr.js # to run just packages/core/integration-tests/test/hmr.js And in case you haven't done that already, before the above, run this to install deps and compile the Rust parts
|
Fixed linter issues
Okay, code is at least working now. Linters and relevant tests passing. The primary issue here was that I forgot to modify the HMR runtime to connect to Only thing left to do is actually write a test that proves #6994 is now solved. |
…g on the same port
Should be now ready to merge. Thanks for guiding me on how to run the tests. Made development a lot easier. |
Any thoughts on when this can get merged? |
# Conflicts: # packages/core/integration-tests/test/proxy.js
Is this something that has to wait until (or is incompatible with) Parcel v3? |
@GeorchW - there is no reason to wait for Parcel v3. This is a simple bug fix as far as I'm concerned. |
↪️ Pull Request
HMR server's WebSocket always upgrades the HTTP server to a WebSocket regardless of the request URL. This patch ensures that the path must start with the
HMR_ENDPOINT
before doing so.Fixes #6994
💻 Examples
None
🚨 Test instructions
Please see issue #6994. Also, this PR needs to be tested before it is merged.
✔️ PR Todo