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

Hot Module Reloading doesn't work after restarting the app #165

Closed
cihadturhan opened this issue Apr 20, 2018 · 10 comments
Closed

Hot Module Reloading doesn't work after restarting the app #165

cihadturhan opened this issue Apr 20, 2018 · 10 comments

Comments

@cihadturhan
Copy link

cihadturhan commented Apr 20, 2018

Do you want to request a feature or report a bug?
Bug
What is the current behavior?
See detail on facebook/react-native#18899

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.
See detail on facebook/react-native#18899

What is the expected behavior?

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

  • Environment:

    • OS: macOS High Sierra 10.13.3
    • Node: 8.11.1
    • Yarn: 1.5.1
    • npm: 5.6.0
    • Watchman: 4.9.0
    • Xcode: Xcode 9.3 Build version 9E145
    • Android Studio: 3.1 AI-173.4670197
  • Packages: (wanted => installed)

    • react: 16.3.0-alpha.1 => 16.3.0-alpha.1
    • react-native: 0.55.2 => 0.55.2
    • metro: 0.30.0 => 0.30.0

More technical details
I made a detailed debugging on how this happens, but I don't know how to solve. This issue happens as a result of two flaws:

  1. HMR Server doesn't know if a client is disconnected (deliberately or not)
  2. When there are multiple clients connected, HMR Server sends the correct updates to only one client

Debugging steps

  1. Prepare a sample project with react-native init ...
  2. Add a breakpoint to attachWebsocketServer@54
  3. Start metro in debug mode, launch the RN app on device, enable HMR on developer menu
  4. Confirm it connects on attachWebsocketServer@54
  5. Change a line in RN, save the file and confirm modules.length ===1 in hmrJSBundle@40
  6. From debug menu on the app, press restart.
  7. Expect the current socket connection close but it never enter attachWebsocketServer.js@71 so the connection lives forever
  8. When app restarts, see it enters in attachWebsocketServer@54 again. This time we will have 2 socket connections from one device.
  9. This means, the line in HmrServer.js@110 will be executed twice
  10. Change a line in RN, save the file. This time [hmrJSBundle@40] will be executed twice. Confirm modules.length === 1 for first websocket (the old one) and modules.length === 0 for the second websocket.
  11. Repeat step 6, 7 and 8 and confirm that you now have 3 websockets and HmrServer will send 3 responses, one of which has non-empty modules, 2 of which has empty modules.

How to solve
ws libary doesn't have a built-in connection close callback. I tried this one as also suggested in this issue
I'm not familiar with the package. I hope this info help you figure the issue out.

@rafeca
Copy link
Contributor

rafeca commented Apr 30, 2018

Thanks for the report! I'm looking into it

@UrbanChrisy
Copy link

Any chance of a status update on this? @rafeca

@DavidNorena
Copy link

@rafeca any update ?

@williamliangwl
Copy link

Hi @rafeca , actually we had similar discussion in the react native repo and people find @cihadturhan 's solution to be working as expected.

Hopefully it can help to solve this issue faster

@gaearon
Copy link
Contributor

gaearon commented Jun 28, 2019

@cihadturhan Any reason you didn't send a PR with your solution?

@cihadturhan
Copy link
Author

@gaearon Ideally a metro should connect more than one client e.g one iPhone simulator, one android emulator and one android phone. My fix removes all the clients and connects just the latest one. So, it’s not a fix. It’s a hack. Besides, I think it doesn’t work anymore.

@gaearon
Copy link
Contributor

gaearon commented Jun 28, 2019

Do repro instructions still work? I can try to figure out a right fix. I don’t know the codebase either but I can try to dig into it if I understand the problem and the workflow you’re trying to achieve better.

@cihadturhan
Copy link
Author

@gaearon no they don’t. It was RN 55.4. There have been many updates and metro code changed a lot. I’m on mobile now so I’m not even sure same websocket library is still in use.

@gaearon
Copy link
Contributor

gaearon commented Jun 29, 2019

So if it doesn’t repro with original instructions then we can consider this fixed?

@cihadturhan
Copy link
Author

I think yes. I haven’t developed RN app for a few months so I’m not sure but last time I tried I couldn’t reproduce.
I’m closing now and will reopen if it’s needed.

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

No branches or pull requests

6 participants