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 reloading breaks on iOS and Android with circular dependencies #17969

Closed
dannycochran opened this issue Feb 13, 2018 · 6 comments
Closed
Labels
JavaScript Resolution: Locked This issue was locked by the bot.

Comments

@dannycochran
Copy link
Contributor

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 6.10.3
Yarn: Not Found
npm: 3.10.10
Watchman: Not Found
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: Not Found

Packages: (wanted => installed)
react: 16.2.0 => 16.2.0
react-native: 0.53.0 => 0.53.0

Steps to Reproduce

Creating a simple circular dependency chain will cause hot reloading to no longer work, and eventually the app will crash with "Maximum call stack size exceeded." This happens on both iOS and Android.

Expected Behavior

  1. Hot reloading reloads the UI in place and can resolve any issues with circular dependencies.
  2. The app does not crash.

Actual Behavior

  1. Hot reloading doesn't work.
  2. For Android, the app will crash with a red screen:

screen shot 2018-02-13 at 10 23 11 am

For iOS, the app will exit without a red screen:

https://drive.google.com/open?id=1BWmmp2yPqtL9zmx4yYQ-yRyrg7nioDhB

Reproducible Demo

Repro steps (feel free to do react-native run-ios as well):

git clone https://github.com/dannycochran/react-native-hot-reloading
cd react-native-hot-reloading
react-native run-android
  1. Enable hot reloading.
  2. Change the text in either "Dependency0.js" or "Dependency1.js", the app should show the "Hot Reloading..." badge, but nothing will change.
  3. On iOS, the app will exit without a red screen. On android, the app should show a red screen with the warning "Maximum call stack exceeded" after a few seconds.

If the app is not crashing, check out the branch with more circular dependencies and repeat the steps above. It almost always crashes for me with the use case on master (which I tried to make as simple as possible), but this branch is a little more complex in terms of the dependency tree, but always fails:

git fetch
git checkout moreCircularDependencies

Similar bugs:

#16613
#7288

@dannycochran
Copy link
Contributor Author

This is fixed on RN master since updating to metro 0.26.0:

b1d8af4

Hopefully it can get patched into 0.54

@K-Leon
Copy link
Contributor

K-Leon commented Feb 20, 2018

@grabbou Can you check if this commit can be cherry picked in 0.54?

@grabbou
Copy link
Contributor

grabbou commented Feb 21, 2018

Yup, see no reason not to update the dependency. I'll wait with the release for a couple of days to make sure I cherry pick everything remaining.

@dannycochran
Copy link
Contributor Author

@grabbou will this end up making it into 0.54?

@grabbou
Copy link
Contributor

grabbou commented Feb 28, 2018

Yup, we've bumped Metro already in latest RC. I think it can be closed in that case.

@grabbou grabbou closed this as completed Feb 28, 2018
@bjornegil
Copy link

In general, circular dependencies should be avoided. Checkout madge for dependency analysis, also pointing out circular dependencies.

@facebook facebook locked as resolved and limited conversation to collaborators Feb 28, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
JavaScript Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

6 participants