React-native: Hot Reloading causes crash on iOS

[alleniver]

Environment

React Native Environment Info:
System:
OS: macOS 10.14
CPU: x64 Intel(R) Core(TM) i5-3210M CPU @ 2.50GHz
Memory: 5.93 GB / 16.00 GB
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 8.12.0 - /usr/local/bin/node
Yarn: 1.12.1 - /usr/local/bin/yarn
npm: 6.4.1 - /usr/local/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 12.1, macOS 10.14, tvOS 12.1, watchOS 5.1
IDEs:
Xcode: 10.1/10B61 - /usr/bin/xcodebuild
npmPackages:
react: 16.6.0-alpha.8af6728 => 16.6.0-alpha.8af6728
react-native: 0.57.4 => 0.57.4
npmGlobalPackages:
react-native-cli: 2.0.1
react-native-git-upgrade: 0.2.7
react-native-rename: 2.3.1

Description

under RN 0.57.2-4, when I turn on Hot Reloading, the iPhone emulator crashes whenever I save the file.The same problem occurs when I create a new RN0.57.4 project. I initially suspected that it was an XCode or emulator problem, but it didn't matter with the XCode version and the emulator version; however, Hot Reloading works fine under RN0.57.1.

[alleniver]

But there is a strange phenomenon, as soon as I turn on Remote Debuging, Hot Reloading is all right, very strange.

[alleniver]

Previous applications were running in the RN0.55.4 environment and everything was fine.

[thiszhong]

+1. After 0.57.1 => 0.57.4

[as27543035]

+1. After 0.57.3 => 0.57.4

[nighttiger1990]

+1

[mikeATL]

+1 bump. I'm having the same issue, do not have the issue in 0.57.2.

[levani]

I'm fighting with this all day long. Now I'm glad I found this issue :D

[narciero]

+1 - yet another example of where a RN release is cut without testing core + essential features

[kelset]

Hey everyone, never noticed this post, and for some reason the bot ignored it too.

So, there are still some information missing to understand why this issue is happening; @alleniver you mention

it didn't matter with the XCode version and the emulator version

what version of XCode have you been able to replicate this with?

Also, which version of Metro do you use?

Did you see any commit from the transition between 0.57.1 to 0.57.2/3/4 that seem to be related to it?

If anyone could create a repro for this issue it would help.

[levani]

@kelset

what version of XCode have you been able to replicate this with?

This happens to me on xcode 10, (with legacy build system enabled, is that matters).

I created a new project with react-native init and could reproduce the issue.

[dslounge]

I wish I could help figure out this issue, but I'm not sure what's going on. Trying a hot module reload crashes here.

This is the call right above. I couldn't find a JSCExecutor.cpp on master.

My info:

  React Native Environment Info:
    System:
      OS: macOS High Sierra 10.13.6
      CPU: (4) x64 Intel(R) Core(TM) i5-4278U CPU @ 2.60GHz
      Memory: 65.13 MB / 8.00 GB
      Shell: 5.3 - /bin/zsh
    Binaries:
      Node: 10.13.0 - ~/.nvm/versions/node/v10.13.0/bin/node
      Yarn: 1.10.1 - /usr/local/bin/yarn
      npm: 6.4.1 - ~/.nvm/versions/node/v10.13.0/bin/npm
      Watchman: 4.7.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 11.4, macOS 10.13, tvOS 11.4, watchOS 4.3
      Android SDK:
        API Levels: 19, 21, 23, 24, 25, 26
        Build Tools: 19.1.0, 23.0.1, 23.0.3, 24.0.3, 25.0.1, 25.0.2, 25.0.3, 26.0.1, 26.0.3, 27.0.3
        System Images: android-28 | Google Play Intel x86 Atom
    IDEs:
      Android Studio: 3.2 AI-181.5540.7.32.5014246
      Xcode: 9.4.1/9F2000 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.6.0-alpha.8af6728 => 16.6.0-alpha.8af6728
      react-native: 0.57.5 => 0.57.5

I created a repo with a simple project where HMR doesn't work: https://github.com/dslounge/rn-57-broken-hmr.

Steps to reproduce:

• download the project, run yarn, yarn start, react-native run-ios
• enable HMR on the simulator
• try to modify anything about src/testComponent.js. The app will crash.

[Liqiankun]

Same issue after upgrade to 0.57.5

[Liqiankun]

Turn on Remote Debuging then not crash @alleniver

[kelset]

Thanks for moving this forward:

• given the comments above, I still believe it's probably easier to understand what was cherry-picked in 0.57.4 (which seems to be the first issue to have this issue) -> https://github.com/react-native-community/react-native-releases/blob/master/CHANGELOG.md#0574

• Here's the commit history (on 0.57-stable) that involves the folder @dslounge screenshot -> https://github.com/facebook/react-native/commits/0.57-stable/ReactCommon/jschelpers

• the fact that it works find when the remote debugging is on is also a hint, maybe it's simply something missconfigured in RN? 🤔

[eoinnorris]

This code is called only when #if Debug is on ( I don't see it on in my version of react which is 0.55).
It happens in the

void JSCExecutor::initOnJSVMThread()

method in JSCExecutor.cpp

if DEBUG

  installGlobalFunction(m_context, "nativeInjectHMRUpdate", nativeInjectHMRUpdate);

endif

[levani]

@kelset

0.57.4 version is broken, not 0.57.3

[kelset]

You are right, I'll edit my comment above - sorry I misread ;)

[alleniver]

@kelset I have tested all the environments, XCode 9.3, 10.0, 10.1, iOS 9, 10, 11, all have this problem, but after starting the remote debugging, there is no problem, the project created by react-native init is the same, very strange.

[alleniver]

@kelset quote"the fact that it works find when the remote debugging is on is also a hint, maybe it's simply something missconfigured? thinking".
i have re-created with react-native init,but the problem still exists.

[dslounge]

I just tried a brand new project using 0.57.3, and trying HMR crashed the app.

Edit:
a brand new install of 0.57.1 works correctly, so does an install of 0.57.2. HMR starts breaking in 0.57.3

Looking at the changelog for 57.3, I saw this change to bump the version of metro: https://github.com/facebook/react-native/commit/2784a03fc52b0b6b241ff8d6ec1fa371ab494e7f#diff-b9cfc7f2cdf78a7f4b91a753d10865a2. I tried to see if bumping the version of metro would make my 0.57.2 project break and it did. I did it by using the "resolutions" field in package.json with yarn.

sample package.json

{
  "name": "rn572",
  "version": "0.0.1",
  "private": true,
  "scripts": {
    "start": "node node_modules/react-native/local-cli/cli.js start",
    "test": "jest"
  },
  "dependencies": {
    "react": "16.5.0",
    "react-native": "0.57.2"
  },
  "devDependencies": {
    "babel-jest": "23.6.0",
    "jest": "23.6.0",
    "metro-react-native-babel-preset": "0.49.2",
    "react-test-renderer": "16.5.0"
  },
  "jest": {
    "preset": "react-native"
  },
  "resolutions": {
    "metro": "^0.48.1",
    "metro-babel-register": "^0.48.1",
    "metro-core": "^0.48.1",
    "metro-memory-fs": "^0.48.1"
  }
}

I verified that downgrading metro to 0.47.1 works in my 0.57.5 project. If you're using yarn you can append this to your package.json to downgrade:

  "resolutions": {
    "metro": "^0.47.1",
    "metro-babel-register": "^0.47.1",
    "metro-core": "^0.47.1",
    "metro-memory-fs": "^0.47.1"
  }

[kelset]

Uhm, nice catch! Can you, instead of relying on 0.47, try to fix the dep to

  "resolutions": {
    "metro": "0.48.2",
    "metro-babel-register": "0.48.2",
    "metro-core": "0.48.2",
    "metro-memory-fs": "0.48.2"
  }

And let me know if it fixes it for you?

[dslounge]

@kelset setting metro to 0.48.2 did not work :(

[cesargdm]

Opening with Xcode and running in debug mode, it crashes at

JSStringRef jsStr = JSC_JSValueToStringCopy(context(), m_value, &exn);

Error message: com.facebook.react.JavaScript (9): EXC_BAD_ACCESS (code=1, address=0x171abda48)

File: Libraries/React.xcodeproj/ReactCommon/jshelpers/Value.cpp/Value::toString()

But it also happens to work when the remote debugger is on.

[kelset]

Thanks everyone, it seems it's something related to Metro 0.48 - this means that probably we should move this issue to the Metro repo. I wonder if, before that, anyway could try moving to latest Metro (0.49.2) (which requires Jest alpha 24.0.0-alpha.6) and double check that this still happens in that scenario.

[charpeni]

Thanks, @kelset.

I tried with the following resolutions:

 "resolutions": {
    "metro": "0.49.2",
    "metro-babel-register": "0.49.2",
    "metro-core": "0.49.2",
    "metro-memory-fs": "0.49.2",
    "jest": "24.0.0-alpha.6"
  },

Unfortunately, it doesn't work. The app won't crash, but the hot reloading does nothing.

_Edit:_ Actually, in a freshly created RN project, it seems to work fine, it may be related to my setup.

[vovkasm]

RN 0.57.5, metro 0.48.3
It works in debug because of this function (metro/src/lib/bundle-modules/injectDelta.js):

function injectDelta(modules) {
  modules.forEach((_ref, i) => {
    var _ref2 = _slicedToArray(_ref, 2);
    let id = _ref2[0],
      code = _ref2[1];
    // TODO(T34661038): This used to support source maps, but I've
    // removed the corresponding code for now since the HmrServer
    // does not generate source maps.

    // In JSC we need to inject from native for sourcemaps to work
    // (Safari doesn't support `sourceMappingURL` nor any variant when
    // evaluating code) but on Chrome we can simply use eval.
    const injectFunction =
      typeof global.nativeInjectHMRUpdate === "function"
        ? global.nativeInjectHMRUpdate
        : eval; // eslint-disable-line no-eval

    injectFunction(code);
  });
}

It calls nativeInjectHMRUpdate only if it is JSC (i.e. not under debugger, because debugger is Chrome).
Next, it calls nativeInjectHMRUpdate with only one argument. But RN blindly expects two args:

#if DEBUG
static JSValueRef nativeInjectHMRUpdate(
    JSContextRef ctx,
    JSObjectRef function,
    JSObjectRef thisObject,
    size_t argumentCount,
    const JSValueRef arguments[],
    JSValueRef* exception) {
  String execJSString = Value(ctx, arguments[0]).toString();
  String jsURL = Value(ctx, arguments[1]).toString();
  evaluateScript(ctx, execJSString, jsURL);
  return Value::makeUndefined(ctx);
}
#endif

This leads to crash.

This works for me (but I don't know how correctly will be source info for loaded scripts without source URL):
ReactCommon/cxxreact/JSCExecutor.cpp

#if DEBUG
static JSValueRef nativeInjectHMRUpdate(
    JSContextRef ctx,
    JSObjectRef function,
    JSObjectRef thisObject,
    size_t argumentCount,
    const JSValueRef arguments[],
    JSValueRef* exception) {
  String execJSString = Value(ctx, arguments[0]).toString();
  String jsURL = argumentCount > 1 ? Value(ctx, arguments[1]).toString() : String();
  evaluateScript(ctx, execJSString, jsURL);
  return Value::makeUndefined(ctx);
}
#endif

[vovkasm]

Some analysis:

• metro was broken in commit: Update the HMR logic to the new IncrementalBundler
• before this commit, working code was: Update the HMR logic to the new IncrementalBundler working part
• metro was fixed in this commit (after 0.48.3): Source map support for HMR

[vovkasm]

Workaround (not fully tested, but fix HMR for me):
Update to metro 0.49.2
Add this to dependencies in package.json:

"devDependencies": {
  ...
  "metro": "0.49.2",
  "metro-react-native-babel-preset": "0.49.2",
  ...
}

then npm install

PS. I'm getting one warning in app after this: unknown call: "relay:check", it seems react-devtools-core should also be updated... (this warning was resolved after update global react-devtools package 3.4.2 -> 3.4.3, it was because inconsistent versions of global react-devtools and application react-devtools-core)

[kelset]

Thanks everyone for the feedback - it seems indeed that in newer Metro the issue is fixed (thanks @vovkasm for the investigation 😊).

Btw does the warning you see disappear if you set Jest to be "jest": "24.0.0-alpha.6"?

I'd really prefer not to force this newer version in 0.57.6 since it requires the Jest alpha, but if it's the 'best way' to fix this issue I may have to, or at least to ask. (since it seems that Jest 24 is still a bit far out)

[vovkasm]

@kelset

Btw does the warning you see disappear if you set Jest to be "jest": "24.0.0-alpha.6"?

Warning I seen, dissapeared after npm install -g react-devtools and run new react-devtools :-)
I'm not update jest, only metro & metro-react-native-babel-preset.

[kelset]

🤦‍♂️ I'm dumb - sorry I misunderstood the PS section. I'll look if I find any commits in master that are related to devtool.

[vovkasm]

I think it is commit Remove Relay classic plugin and any reference to Relay

[kelset]

Ah nice catch, I actually found one which is strictly related to ReactDevTools -> Fix React Native AsyncMode and DevTools

[kk412027247]

After turning on the Debug JS Remotely, there is no more crash on IOS.
This approach is temporary, but it work at "react-native": "0.57.5".

[sryze]

Still crashes in 0.57.5

[vovkasm]

@kelset Updating metro to 0.49.2 will broke remote debug in chrome:

I think this exception can be easily fixed but the process of this will be long: it needs to be fixed, then new version of metro should be rolled, than it should be moved to RN... too long.

So I returned to my original solution described here https://github.com/facebook/react-native/issues/22116#issuecomment-440420713

This changes native code so that it will not crash. So question. Will you plan a new patch release of RN 0.57 if I send PR against 0.57-stable branch (can such PR be accepted? because I can't patch master - this code not exists anymore in master)

UPDATE: If anyone want to test my above solution, you can set my local fork as dependency in package.json (note: you can do this easily only for iOS, for Android you will need to modify build settings to build RN with application):

 "react-native": "git+https://github.com/vovkasm/react-native.git#v0.57.5-woof.1",

[alleniver]

@vovkasm nice job!

thanks all.

[kelset]

Hey @vovkasm thanks for the update!

(can such PR be accepted? because I can't patch master - this code not exists anymore in master)

Yes I can accept & merge PRs towards the branch, I was just wondering if we can find the commit that removes the code that you modified before - because maybe there is a better solution to this hidden in the commits and it's easier to just cherry pick that ;)

Also it makes me wonder if this fix will work with both Metro 48 and 49.

[vovkasm]

@kelset

Also it makes me wonder if this fix will work with both Metro 48 and 49.

Yes. My fix totally backward and forward compatible. It simple fix crash if JS call function nativeInjectHMRUpdate with one argument. If JS will call nativeInjectHMRUpdate with two arguments it will behave as before and count source of code (the second argument).

if we can find the commit that removes the code that you modified before

I was tried to search nativeInjectHMRUpdate in new jsi code from master, but found nothing. I don't know why. May be because eval (https://github.com/facebook/metro/blob/v0.49.2/packages/metro/src/lib/bundle-modules/injectUpdate.js#L19) supposed to work with new jsi may be it simple wasn't implemented... (
Anyway commit that removed nativeInjectHMRUpdate is this: https://github.com/facebook/react-native/commit/7ffb406517e70a2ac299d20020d3798a5b7dc612

[kelset]

Ah awesome! Then yeah I think that we can proceed without too many problems with a PR to the branch. I was planning to release 0.57.6 today, but it can wait if you think you can create the PR in the next few days

[vovkasm]

Done #22412

[otaro808]

+1 after 0.57.0 => 0.57.5

[vovkasm]

Should be fixed in 0.57.6

[sryze]

Did this patch actually fix hot reloading? I think it's not working properly in 0.57.7, although doesn't crash anymore. In my case there is a "Hot Reloading..." notification shown but the screen is not reloaded (maybe it's a different bug?)

[vovkasm]

@sryze Yes, it actually fix hot reloading. I checked HMR enabled with remote debugger and without. It actually work. Can you check with empty project after react-native init?

[xanderberkein]

Can confirm, hot reloading broke again in 0.57.7

[vovkasm]

Please provide sample repo on github and steps to reproduce broken HMR.
Note also, that this bug is about concrete crash when HMR enabled, not about it didn't work.

[charpeni]

This may be not directly related to this issue, but I can confirm that HMR could be broken as stated in my previous comment: https://github.com/facebook/react-native/issues/22116#issuecomment-440320895.

I have updated an existing RN app to 0.57.7, the crash issue has been solved. Unfortunately, now I can only see the HMR banner pop in and out without actually making any visible changes.

On a freshly created RN 0.57.7 project, I do not have any issue, only on an updated one.

I do not have any reproducible steps, I'm trying to reproduce it in a sample repo.

[Titozzz]

I updated to 0.57.7 and had no issue 🤔 , should I reopen this or maybe we need to create another issue ?

[kelset]

I feel that it would be best to open a new issue if we can get to a stage where there is a repro.

[charpeni]

If you're still experiencing issues with the Hot Module Reloading after upgrading to [email protected], it's most likely due to a cache error or related to your dependencies' lock file.

In case a step doesn't resolve your issue, proceed with the next one.

Step 1

The first step will be to check if you have a .git/index.lock file, if it's the case, this file will prevent watchman to trigger HMR. You can safely delete this file.

Step 2

You can delete all cache files, clear watchman, and remove your dependencies with:

rm -rf $TMPDIR/react-* && rm -rf $TMPDIR/metro-* && rm -rf $TMPDIR/haste-* && watchman watch-del-all && rm -rf node_modules/

Then, start react native with:

yarn start --reset-cache

Step 3

If you're here, it's most likely due to outdated dependencies in your dependencies' lock file.

I'd recommend you to delete your yarn.lock or package-lock.json and reinstall all dependencies to regenerate a new lock file.

⚠️ This step may not be safe, as you're not guaranteed to have all exact and working dependencies as before, but it's a good step to find what's going on with your dependencies.

Then, start react native with:

yarn start --reset-cache

If everything is working fine, you can compare changes in your lock file to find what was causing this headache or let it like this. Otherwise, let me know.

[xanderberkein]

Seemed to be an issue with outdated dependencies indeed!

Since I didn't feel safe deleting my yarn.lock file on a production app, I ran yarn upgrade on all my babel packages, which resolved the hot reload issue.

[psolom]

@charpeni I went through all the steps, but still get unknown call: "relay:check" with the following crash on reload, unfortunately. My environment: https://github.com/facebook/react-native/issues/22349#issuecomment-443846352