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

[HMR] Ensure Hot Loading is working on Android #5338

Closed
martinbigio opened this issue Jan 15, 2016 · 27 comments
Closed

[HMR] Ensure Hot Loading is working on Android #5338

martinbigio opened this issue Jan 15, 2016 · 27 comments
Labels
Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.

Comments

@martinbigio
Copy link
Contributor

We've done quite many changes to HMR since @satya164 added the Hot Loading option to the dev menu. This task is to make sure Hot Loading works on a brand new project. Also, we need to gate the feature for now, we don't want to release up until we're sure it's stable.

@facebook-github-bot
Copy link
Contributor

Hey martinbigio, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!

@martinbigio martinbigio changed the title Ensure Hot Loading is working on Android [HMR ]Ensure Hot Loading is working on Android Jan 15, 2016
@martinbigio martinbigio changed the title [HMR ]Ensure Hot Loading is working on Android [HMR] Ensure Hot Loading is working on Android Jan 15, 2016
@JohnyDays
Copy link
Contributor

Using the latest v0.21.0-rc I can't seem to figure out how to enable HMR, should there be a dev menu option?
I simply ran react-native init then npm install --save react-native@0.21.0-rc.

@satya164
Copy link
Contributor

@JohnyDays For now you need to install from master - http://facebook.github.io/react-native/docs/android-setup.html#content

@JohnyDays
Copy link
Contributor

Thanks, I'll give it a look and give some feedback 👍

@satya164
Copy link
Contributor

@JohnyDays 😄

@JohnyDays
Copy link
Contributor

For anyone stuck in making it work, there is no need to install from master, here are the steps for a new project:

  1. react-native init ProjectName && cd ProjectName
  2. npm install --save react-native@0.21.0-rc
  3. react-native upgrade (Choose yes for everything it asks)
  4. You're done! An option to enable hot reload should appear in the android dev menu

@satya164
Copy link
Contributor

Oh! I thought @martinbigio removed it before the RC. Finally we have HMR for Android in a public release 🎉

@JohnyDays
Copy link
Contributor

It seems to work perfectly, but everytime a change happens, this warning pops up
screenshot from 2016-02-24 07-27-10

And it keeps stacking up at the bottom of the app, so if I save the file 5 times, it appears 5 times (consistently)

EDIT: Actual chrome stacktrace:

Possible Unhandled Promise Rejection (id: 0):
Network request failed
TypeError: Network request failed
    at xhr.onload (http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:5883:8)
    at XMLHttpRequest._sendLoad (http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:19736:1)
    at XMLHttpRequest.setReadyState (http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:19726:6)
    at XMLHttpRequest._didCompleteResponse (http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:19621:6)
    at http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:19575:105
    at EventEmitter.emit (http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:17889:23)
    at MessageQueue.__callFunction (http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:16519:23)
    at http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:16423:8
    at guard (http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:16377:1)
    at MessageQueue.callFunctionReturnFlushedQueue (http://localhost:8081/index.android.bundle?platform=android&dev=true&hot=true:16422:1)

@JohnyDays
Copy link
Contributor

Figured out the problem.
The source maps are being requested from :

http://localhost:8081/index.android.map?platform=android&runModule=false&entryModuleOnly=true&hot=true

It should probably be requested from the remote url/ip (e.g 192.168.1.183).
Is this packager related, or android-specific?
At the end of the generated HMR patch, the sourcemaps are referenced as

//# sourceMappingURL=/index.android.map?platform=android&dev=true&hot=true

UPDATE: If debugging in chrome, changing the url from localhost:8081 to $YOUR_IP:8081 will stop this warning. But I haven't found a way to do this if you're not debugging on chrome Nevermind, doesn't look like it worked very well

@martinbigio
Copy link
Contributor Author

Hmm though @mkonicek removed it. It wasn't intended to have the option visible on the menu yet

@martinbigio
Copy link
Contributor Author

Ohh, this is 0.21RC. We'll remove the option from the menu there. If everything goes well we should be able to open the feature on 0.22 :)

@martinbigio
Copy link
Contributor Author

@JohnyDays Thanks so much for working on adding support for having multiple clients!. Please let us know if you find any bug or if there're other feature requests you think we should implement. We'll need help on testing this, so the more people use it before it's open the better :)

@JohnyDays
Copy link
Contributor

I'm having some trouble getting the android version to compile my java code changes.

I have react-native cloned into the node_modules folder, and when I run react-native run-android it doesn't seem to pick up on them, regardless of me changing branches or whatever I do.
I tried doing cd android && ./gradlew clean but had the same result. Is there something obvious I'm missing?

As an aside, symlinking react-native from another folder makes it so the packager doesn't correctly serve the files

@satya164
Copy link
Contributor

@satya164
Copy link
Contributor

Symlinking won't work, coz watchman, which RN uses to watch files, doesn't support symlinks.

@JohnyDays
Copy link
Contributor

Yeah I have my machine (arch-linux) setup with the sdk, and gradle daemon, all the build tools and genymotion.
The builds always succeed, my problem seems to be related to the file system changes not being picked up.
I started a new project and it works perfectly again, it's probably a linux-interop problem.

@JohnyDays
Copy link
Contributor

Shaping up! Still get those warnings though 😢

optimized

PS: I just spent way more time than I should trying to make that gif under 10mb, sorry for the terrible colors

@JohnyDays
Copy link
Contributor

@martinbigio Just a heads up, i reimplemented the changes to attachHMRServer.js from b2b41da and 20588a6, so there wouldn't be any problems with conflicts. I should be able to open the PR today, cheers 🍺

@martinbigio
Copy link
Contributor Author

Wow, this indeed will be helpful for a lot of people!.

I recently added support for redux stores and modules that export pure functions and modified this file (436db67). Most likely you'll have to rebase.

@NishanthShankar
Copy link

Just updated to rn-0.22 and the warnings(as shown by @JohnyDays) are still there

@th0th
Copy link

th0th commented Mar 22, 2016

Yep, I am also getting this on genymotion with react-native 0.22. I also tried setting debug server host & port to point to my development machine, no luck.

EDIT: When I edit and save a file on the editor I see 'Hot Loading...' toast on genymotion. I guess application is aware of the change but fails to load.

screen shot 2016-03-22 at 09 43 58

@JohnyDays
Copy link
Contributor

@martinbigio I did some digging on this when i was working on multiple clients, and it appears to be related to source maps. e.g even if you are requesting from a remote ip, the source maps still point to localhost.

@martinbigio
Copy link
Contributor Author

Genymotion sets a special ip to access localhost from the simulator, however to fetch the sourcemaps we're using localhost...

@martinbigio
Copy link
Contributor Author

The problem is that JS lacks of this logic:

if (isRunningOnGenymotion()) {
return GENYMOTION_LOCALHOST;
}

cc @mkonicek

@JohnyDays
Copy link
Contributor

Shouldn't it be the java sending this information to the packager? Such as genymotion=true or localhost=10.0.3.2 in the url

@martinbigio
Copy link
Contributor Author

Yup, we're going to expose this on the AndroidConstants native module. @satya164 will send a PR in a couple of hours :)

@ghost ghost closed this as completed in 6c22a21 Mar 24, 2016
mkonicek pushed a commit that referenced this issue Apr 1, 2016
Summary:Source maps are broken on Genymotion right now as they aren't being loaded from the correct URL. refer - #5338 (comment)

**Test plan**

Build and install UIExplorer from master branch in genymotion and enable hot reload. When you change a file and save it, you'll see a Yellow box due to source map fetching failed, as per the referenced comment.

Doing the same for this branch doesn't produce any yellow boxes.
Closes #6594

Differential Revision: D3088218

Pulled By: martinbigio

fb-gh-sync-id: 0d1c19cc263de5c6c62061c399eef33fa4ac4a7b
shipit-source-id: 0d1c19cc263de5c6c62061c399eef33fa4ac4a7b
@suhaotian
Copy link

same error as above still

react-native version:
➜ AwesomeReactNative git:(master) ✗ react-native -v
react-native-cli: 0.2.0
react-native: 0.22.2

zebulgar pushed a commit to nightingale/react-native that referenced this issue Jun 18, 2016
Summary:Source maps are broken on Genymotion right now as they aren't being loaded from the correct URL. refer - facebook#5338 (comment)

**Test plan**

Build and install UIExplorer from master branch in genymotion and enable hot reload. When you change a file and save it, you'll see a Yellow box due to source map fetching failed, as per the referenced comment.

Doing the same for this branch doesn't produce any yellow boxes.
Closes facebook#6594

Differential Revision: D3088218

Pulled By: martinbigio

fb-gh-sync-id: 0d1c19cc263de5c6c62061c399eef33fa4ac4a7b
shipit-source-id: 0d1c19cc263de5c6c62061c399eef33fa4ac4a7b
cpojer pushed a commit to facebook/metro that referenced this issue Jan 26, 2017
Summary:Source maps are broken on Genymotion right now as they aren't being loaded from the correct URL. refer - facebook/react-native#5338 (comment)

**Test plan**

Build and install UIExplorer from master branch in genymotion and enable hot reload. When you change a file and save it, you'll see a Yellow box due to source map fetching failed, as per the referenced comment.

Doing the same for this branch doesn't produce any yellow boxes.
Closes facebook/react-native#6594

Differential Revision: D3088218

Pulled By: martinbigio

fb-gh-sync-id: 0d1c19cc263de5c6c62061c399eef33fa4ac4a7b
shipit-source-id: 0d1c19cc263de5c6c62061c399eef33fa4ac4a7b
@hramos hramos added Help Wanted :octocat: Issues ideal for external contributors. and removed Help Wanted :octocat: Issues ideal for external contributors. labels Mar 8, 2018
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

10 participants