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

Fix for #6720: HMR not working in Firefox if proxy option present #7444

Merged
merged 3 commits into from
Aug 7, 2019
Merged

Fix for #6720: HMR not working in Firefox if proxy option present #7444

merged 3 commits into from
Aug 7, 2019

Conversation

dmile
Copy link
Contributor

@dmile dmile commented Jul 28, 2019

Original issue: #6720
Root cause: sockjs/sockjs-client#474 (comment)
This is a PR for naive idea how to fix it: #6720 (comment)

Copy link
Contributor

@mrmckeb mrmckeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide testing criteria for this please @dmile?

[Edit] A test repository would actually be ideal.

@iansu can I get your thoughts on this?

@dmile
Copy link
Contributor Author

dmile commented Jul 28, 2019

Can you provide testing criteria for this please @dmile?

[Edit] A test repository would actually be ideal.

Testing is pretty simple, not sure that special repository needed:

  1. Add a proxy field to your package.json, for example: "proxy": "http://localhost:4000"
  2. Run the app in the development mode. Open http://localhost:3000 to view it in the Firefox.

Errors like:

  • The development server has disconnected ... in browser console
  • Proxy error: Could not proxy request /sockjs-node/947/wwr0adlx/websocket from localhost:3000 to http://localhost:4000 ... in app console (if you make code edits)

should disappear

Copy link

@patilharshal16 patilharshal16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the hard-coded path is a good approach?

@dmile
Copy link
Contributor Author

dmile commented Jul 31, 2019

@patilharshal16 For now, there is no way to configure it. This path is more like a constant than variable:

// Hardcoded in WebpackDevServer
pathname: '/sockjs-node',

P.S. Webpack has special sockPath option. But I don't know how to pass it to webpackHotDevClient and WebpackDevServerUtils. Any suggestions?

Copy link

@patilharshal16 patilharshal16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case you need to check for platform compatibility for this path.

@dmile
Copy link
Contributor Author

dmile commented Aug 3, 2019

@iansu

can I get your thoughts on this?

@patilharshal16

the hard-coded path is a good approach?

I've created a branch with configurable socket path. Based in this #6280 (comment).
if current bugfix will be approved, I'll create the new PR.

@mrmckeb mrmckeb merged commit eaf0b17 into facebook:master Aug 7, 2019
@lock lock bot locked and limited conversation to collaborators Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants