React-native-debugger: iOS - Touch events blocked when debugger is attached

[freerangenerd]

This appears to be a bug with the debugger.

React Native Debugger app version: 0.10.4
React Native version: 0.59.8
Platform: iOS
Is real device of platform: Reproduces in simulator and on devices
Operating System: iOS app, debugging on MacOS 10.14.6.
Generally, devs are using iOS 12 variants in the simulator.

Noted.

Issue:

I work on a commercial application that has been using the React Native Debugger for at least two years. When we upgraded to React Native 0.59.8 we found that suddenly touch events are not seen by the application in React Native views when the React Native Debugger is attached.

Touch events work fine in native views.

We have resorted to telling our devs to use Chrome Tools to debug, but many would still prefer to use the React Native Debugger.

If the debugger is not attached the application operates normally.

[jacklj]

I'm seeing this bug too - on the iOS simulator, when the debugger is attached, you can't click on anything in the app. I'm not sure if it's related, but the React component tree section slows down until about 15 seconds since the app launched, and then it crashes (showing "Waiting for React to connect...". The app also crashes out with "Runtime is not ready for debugging".

[erlong16]

What I see happening is iOS native sees the tap event and tracks it to the correct UI item. The touch event is sent to the JS side from RCTTouchHandler.m, _updateAndDispatchTouches:eventName.

It is received in ReactNativeRenderer-prod.js and eventually lands here:

https://github.com/facebook/react-native/blob/159624d901c8143999e1ce59b852080019b528f3/Libraries/Renderer/implementations/ReactNativeRenderer-prod.js#L1005

This line gets back null when React Native Debugger is attached because instanceCache is empty:

inst = getInstanceFromTag(rootNodeID)

When using the Chrome debugger instead, instanceCache has all tagged UI elements, including the item that was tapped, so it returns the correct value for inst.

When RN debugger is attached I see precacheFiberNode() called for the expected UI items.
https://github.com/facebook/react-native/blob/159624d901c8143999e1ce59b852080019b528f3/Libraries/Renderer/implementations/ReactNativeRenderer-dev.js#L2640

instanceCache eventually has all the items cached. But when the touch event is received, it is empty. I haven't determined what resets or clears it.

Hrm... just notice the links point to different files: ReactNativeRenderer-prod.js and ReactNativeRenderer-dev.js.

Interesting. When using the Chrome Debugger touch events come to ReactNativeRenderer-dev.js. When using React Native Debugger touch events come to ReactNativeRenderer-prod.js.

[UPDATE]

So the gist of the bug is that the instanceCache that is updated with all the instances is the one in ReactNativeRenderer-dev.js, but the one used to try and look up the item tapped is in ReactNativeRenderer-prod.js. It's using the event handler registered from here:

https://github.com/facebook/react-native/blob/159624d901c8143999e1ce59b852080019b528f3/Libraries/Renderer/implementations/ReactNativeRenderer-prod.js#L1040

But it should be using the one from here:

https://github.com/facebook/react-native/blob/159624d901c8143999e1ce59b852080019b528f3/Libraries/Renderer/implementations/ReactNativeRenderer-dev.js#L2979

[erlong16]

This line is bringing in the prod renderer:

https://github.com/jhen0409/react-native-debugger/blob/master/app/worker/utils.js#L39

Locally, if I add this before it, things work correctly:

if (__DEV__ && moduleId === 138) {
continue;
}

If it calls metroRequire on it, things don't work. Since that moduleId can't be hard-coded like that, I'm note yet sure how to determine which moduleId needs to be skipped.

[elie222]

Seeing the same issue on an RN 0.60 project.

[TheSavior]

Disclaimer: I haven't fully read through this issue...

It shouldn't be possible to get into the prod renderer in dev. All callsites wanting to access the renderer should go through this file which picks the correct renderer based on DEV. https://github.com/facebook/react-native/blob/master/Libraries/Renderer/shims/ReactNative.js

If there are any callsites that are requiring the renderer directly that is probably a bug and should instead be going through that shim file.

[erlong16]

React Native Debugger's lookupForRNModules() is causing the prod renderer to load, as noted above. It may be that other prod variants are loading in DEV as a result of this as well.

"Shouldn't be possible" is less accurate than "shouldn't be". The dev variants do have a check to make sure __DEV__ is true before executing to ensure safety. It seems reasonable that the prod variants should have a check to make sure it is false.

[TheSavior]

Wrapping it in a !DEV seems like it addresses the symptoms but not the root cause. Something is trying to cause two renderers to be loaded which is going to cause a ton of other unknown problems.

[erlong16]

It is hard to argue with that. React Native Debugger is loading every module to inspect it. There may be other side effects.

As far as prod vs. dev variants go, the only other case I see in the source is in ReactFabric.js.

[cdsanchez]

It would be useful to try to understand why RN Debugger is scanning modules, and then maybe FB can provide guidance on whether there's a better way to accomplish whatever the goal may be.

A superficial explanation from 5 minutes of looking at the code is that it needs the 'react-native' module to hook up some dev utils, but because metro is eliding module names in favor of module ids (i.e. preventing the lookup by name), they resort to scanning module ids 0..1000 looking for a module that "looks like" react-native (i.e. it exports requireNativeComponent and NativeModules).

[erlong16]

Right. It is a rock vs. hard place problem. Hopefully there is a better way.

[erlong16]

To summarize the problem...

When React Native Debugger is setting up during launch, it needs to get references to four modules instances in the bundle of the target application being debugged:

'MessageQueue'
'NativeModules'
'Platform'
'setupDevtools'

These are obtained here in its source:

https://github.com/jhen0409/react-native-debugger/blob/master/app/worker/utils.js

In this file, const getRequiredModules is called.

getRequiredModules accesses a const requiredModules.

requiredModules calls on const getModule for each of the four module instances it needs, passing the name to getModule along with the count of moduleIDs in the bundle.

getModule calls lookupForRNModules to get a reference to the 'reactNative' module - which it identifies as an object with requireNativeComponent and NativeModules defined.

lookupForRNModules gets a metroRequire function from window.r or window.require.
It then loops over all moduleIDs calling metroRequire on each ID until it finds a module with requireNativeComponent and NativeModules defined.
It then returns that module to getModule.

getModule takes the returned 'reactNative' module and gets the value for the name of the module it was called to load from it: "reactNative[name]"
It returns that value to requiredModules.

lookupForRNModules is causing problems by requiring modules that shouldn't be required. The specific problem identified is that it loads ReactNativeRenderer-prod.js when __DEV__ is true, which should never happen. There may be other side effects not identified.

To fix this, the question is:

How can lookupForRNModules identify the 'reactNative' module without iterating over moduleIDs, loading the module, and checking if both requireNativeComponent and NativeModules are defined?

[TheSavior]

I'm not sure the question you are asking is the right question.

Why do you need to reach into RN core internals in the first place?

Instead of reaching in, could you have the setup for this module be a couple of lines that people add to their bundle that expose the information you need in a place you know you can access it?

[erlong16]

React Native Debugger is a general application completely external from the app being debugged over the bundle server connection. Developers using it don't add code to their apps to work with it.

I didn't develop React Native Debugger. I don't know the challenges it faces interacting with an application being debugged like this. It seems more complicated than what the React Native application being debugged has to deal with in accessing modules. I can only observe from the debugger source what it needs. I don't know how it can better achieve its ends given its context.

The repo owner has not commented on this issue. I sent him a direct email with no reply.
Since this tool is important to developers using it, I am trying to determine what can be done in hopes of submitting a pull request.

[cdsanchez]

I had another 5 minutes to look into this.. but it looks like this tool is using ReactNative.NativeModules to facilitate useful developer actions like reloading RN, introspecting AsyncStorage, etc. from the desktop app UI.

See here: https://github.com/jhen0409/react-native-debugger/blob/9c77d95ea8ec842a41210d6da24afd965225862f/app/utils/devMenu.js#L74-L89

From a developer experience point of view these seem like legitimate needs. However, the implementation seems "sub-optimal" because it uses potentially unstable APIs to implement this. At the same time, it doesn't seem that there would have been any other way for the developer to implement this without requiring setup code in the app from what I understand.. so I can see why they did it this way.

If React Native wants to allow tools to poke and prod the runtime like this responsibly as a means of enabling DX-improving tools without any setup code, it seems that there should be some stable and easily accessible runtime tooling API provided by RN. By accessible, I mean in a way that doesn't depend on the specific bundler that was used (i.e. it should work with Metro, Haul, etc.). The KISS solution here would probably be some JS global set upon initialization of React Native.

i.e. some variant of:

window.__ReactNative__ = require('react-native);
// alternatively, expose some "stable" interface of developer functionality

Personally, I think this is a fair trade-off, but I'm usually wrong :)

The alternative solution suggested by @TheSavior could look something like:

// RN app entry point
import setup from 'react-native-debugger';

setup();

This is reasonable, but I personally prefer reducing the amount of boilerplate required by developers. Additionally, it'll require an action on the part of those already using this tool.

[TheSavior]

It sounds like this projects goals and scope are sufficiently similar to what we plan on supporting out of the box for React Native with Flipper in 0.62+ that I'm curious if @rickhanlonii can chime in on some implementation thoughts.

[jhen0409]

I think the issue was fixed by https://github.com/jhen0409/react-native-debugger/commit/490cbaafc06f646f98f4673508e72f645664979c, it wouldn't use metroRequire for uninitialized module on React Native Debugger v0.10.7.

@cdsanchez, I'm sorry I might losing the email. You can directly @ me on this issue.

[abdulghanitech]

This is issue still there in v0.11.3. I'm trying with Android device.