Skip to content

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

Merged
mrmckeb merged 3 commits into
react:masterfrom
dmile:fix-hmr-for-firefox
Aug 7, 2019
Merged

Fix for #6720: HMR not working in Firefox if proxy option present#7444
mrmckeb merged 3 commits into
react:masterfrom
dmile:fix-hmr-for-firefox

Conversation

@dmile

Copy link
Copy Markdown
Contributor

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

@mrmckebmrmckeb left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
ContributorAuthor

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

@patilharshal16patilharshal16 left a comment

Copy link
Copy Markdown

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
Copy Markdown
ContributorAuthor

@patilharshal16 For now, there is no way to configure it. This path is more like a constant than variable: https://github.com/facebook/create-react-app/blob/b96ac6c2c007db85812a6e9c0ec54509019c0fea/packages/react-dev-utils/webpackHotDevClient.js#L66-L67
P.S. Webpack has special sockPath option. But I don't know how to pass it to webpackHotDevClient and WebpackDevServerUtils. Any suggestions?

@patilharshal16patilharshal16 left a comment

Copy link
Copy Markdown

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

dmile commented Aug 3, 2019

Copy link
Copy Markdown
ContributorAuthor

@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 react:masterAug 7, 2019
@locklockBot locked and limited conversation to collaborators Aug 12, 2019
Sign up for freeto 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

@dmile@iansu@mrmckeb@patilharshal16@facebook-github-bot