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

Add device.reactNativeDebugRemotely(bool) #637

Closed
Salakar opened this issue Mar 24, 2018 · 9 comments
Closed

Add device.reactNativeDebugRemotely(bool) #637

Salakar opened this issue Mar 24, 2018 · 9 comments

Comments

@Salakar
Copy link

Salakar commented Mar 24, 2018

Description

Would love to be able to toggle JS remote debugging programatically via the detox device api - much like device.reloadReactNative(). My use case is as discussed here: #470 (comment)

What are your thoughts on adding in this functionality? Is this something you'd be against, it should be fairly trivial change wise and I've curated and tested the snippets required to toggle it on each platform if it helps.

I could attempt to PR it - but might take me a while compared to someone who's familiar with the detox code base 🙈 - but if not I'll give it a go 😛

Proposal api: - ?

await device.reactNativeDebugRemotely(boolean);

Android - Programatically toggle debugging

getReactNativeHost()
    .getReactInstanceManager()
    .getDevSupportManager()
    .getDevSettings()
    .setRemoteJSDebugEnabled(true); // or false

Tested and confirmed this in my MainApplication.java -> onCreate override.


iOS - Programatically toggle debugging

import { NativeModules } from 'react-native';

// only on iOS
NativeModules
    .RCTDevSettings
    .setIsDebuggingRemotely(true); // or false

Which resolves to this native code here: https://github.com/facebook/react-native/blob/master/React/Modules/RCTDevSettings.mm#L298


@LeoNatan
Copy link
Contributor

I cannot say I am really inclined to like this proposal.

We've had issues with reloadReactNativeApp() as it is. Adding more RN-specific API usually entails supporting it in a very moving target. The snippets provided are JavaScript code, but the client side of Detox is native only. We do not inject JavaScript in the client. And RN native changes a lot.

Also, what is the use-case here? We had a request for something in the area of this functionality, but that was from FB RN-core developers. They wanted to test the dev menu itself, so we added shake support to the device API. But what is the use-case for remote debug?

@LeoNatan LeoNatan changed the title [RN][Request] device.reactNativeDebugRemotely(bool) device.reactNativeDebugRemotely(bool) Mar 25, 2018
@LeoNatan LeoNatan changed the title device.reactNativeDebugRemotely(bool) Add device.reactNativeDebugRemotely(bool) Mar 25, 2018
@Salakar
Copy link
Author

Salakar commented Mar 26, 2018

@LeoNatan my original issue description does include the native code though for both platforms and also my use case as well 🙈

I do get the hesitation around additional RN functionality though as RN internal APIs seem to change every release. I've had issues myself with the reload functionality being flakey and had to swap it out for react-native-restart internally for the time being until I can debug.

I think with a couple of people keeping on top of the RN releases this should be ok to keep these sorts of features going - something I can assist with?

@LeoNatan
Copy link
Contributor

Sorry, I missed the link to the comment.

Would you like to work on a PR for this feature?

@Salakar
Copy link
Author

Salakar commented Mar 26, 2018

@LeoNatan ye sure I can give it a go - just wanted to make sure it was something you guys would consider before I invested the effort into PR'ing

@LeoNatan
Copy link
Contributor

We are still considering how to handle community contributions properly. We don’t want to reach a place where we’ve accepted requests but then there is no one to support these features after the fact. Before you start work, let’s see what @rotemmiz has to say.

@Salakar
Copy link
Author

Salakar commented Mar 26, 2018

@LeoNatan If it helps re support - it would be a fundamental part of our testing suite for react-native-firebase so would be something I'd be invested in helping to keep it working. (our new WIP testing setup with bridge + detox is here)

@LeoNatan
Copy link
Contributor

Such a commitment does help a lot, thank you!

Another issues worth considering is if the RN remote debugging works in release build. We normally recommend running Detox on release builds because they are closer to what the user experiences from an app. But release builds do not use the packager, which I think is necessary for remote debugging. I could be wrong.

@jaceofslades
Copy link

This is an issue we're dealing with at the very moment is that with a number of changes to our API, remote debugging would be very useful in order to verify that our mocks match what would be returned by the API.

@LeoNatan
Copy link
Contributor

There has been no traction on this, and as I said before, I don't see us implementing support for this. Closing. If a PR is submitted, we will be willing to consider.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants