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

Remove remote debugging from dev menu #36754

Closed

Conversation

gabrieldonadel
Copy link
Collaborator

Summary:

This PR proposes the removal of the ability to use remote JS debugging (a.k.a Chrome Debugging) from the DevMenu.
This change has been suggested previously on the contributor's Discord server and it is motivated by the fact that generally speaking, this feature does not work with the new architecture and most of the popular modules these days. The remote JS debugging is basically broken if you use Turbo Modules, Fabric, or sync native module calls.

People tend to assume that if something is part of the dev tools UI, it is going to work reliably. However, when it comes to remote debugging using JSC that's increasingly unlikely to be the case. Having it continue to exist as an option has created some confusion, or at least that's what we've seen within the Expo community.

How to manually enable remote debugging

If your project depends on remote debugging, you can still enable it manually through the NativeDevSettings. E.g.

import NativeDevSettings from 'react-native/Libraries/NativeModules/specs/NativeDevSettings';

export default function App() {
  return (
    <Button
      title="Enable remote debugging"
      onPress={() => NativeDevSettings.setIsDebuggingRemotely(!true)}
    />
  );
}

Next steps

Once this has been released and we are sure that this doesn't cause a lot of problems for people we can remove the ability entirely in a follow-up PR

Changelog:

[General] [Removed] - Remove remote debugging from the dev menu

Test Plan:

Locally run rn-tester using JSC on Android and iOS and open the dev menu

iOSAndroid

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 31, 2023
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,567,303 -2,943
android hermes armeabi-v7a 7,881,701 -2,930
android hermes x86 9,049,233 -2,932
android hermes x86_64 8,904,248 -2,956
android jsc arm64-v8a 9,165,989 -2,019
android jsc armeabi-v7a 8,356,403 -2,013
android jsc x86 9,222,671 -2,013
android jsc x86_64 9,480,497 -2,019

Base commit: 2ab750f
Branch: main

@motiz88
Copy link
Contributor

motiz88 commented Apr 1, 2023

cc @huntie, @voideanvalue who are working on debugger integrations

@motiz88
Copy link
Contributor

motiz88 commented Apr 1, 2023

Personally I think we should keep this under the Old Architecture and only remove it for the New Architecture. cc also @cortinico for thoughts.

@cortinico
Copy link
Contributor

Personally I think we should keep this under the Old Architecture and only remove it for the New Architecture. cc also @cortinico for thoughts.

That's a good alternative to me. We'll be looking into alternatives for debugging within the New Architecture, so we might need to revive this solution (or a different one) in the near future.
But I totally agree with the intention that whatever shows up on the Dev Menu should reliably work otherwise it just creates more frustration.

@cortinico cortinico added p: Expo Partner: Expo Partner labels Apr 3, 2023
@brentvatne
Copy link
Collaborator

I'm in favor of removing this option entirely, or if we do keep it, then we should update the text to indicate that it is unlikely to work. Most projects use modules that are incompatible with remote debugging these days, and I think it creates a bad experience to provide this as an option when it doesn't typically work.

@motiz88
Copy link
Contributor

motiz88 commented Apr 5, 2023

Most projects use modules that are incompatible with remote debugging these days

@brentvatne could you clarify? My understanding is that the new architecture doesn't support remote debugging - are there popular modules that break debugging under the old architecture, as well? Or were you referring mainly to the difference between the new and old architecture?

More generally, can we put together a small matrix of supported configurations of RN and make sure we have a working debugging workflow for each one? e.g. for the following combinations of architecture and engine (which are all still supported in RN, AFAIK), I believe these are the best options we have to offer: (Please correct me where I have the facts wrong / missing)

New arch Old arch
Hermes Direct debugging over CDP Direct debugging over CDP
JSC Direct debugging with Safari? Remote debugging?

My main point with regards to this PR is that we shouldn't take away remote debugging in configurations where there's no current alternative. Totally happy for us to remove it where it's properly superseded by something better, like direct debugging, but I'm not sure if we currently have the information required to render the appropriate dev menu for each configuration.

For context, @huntie is currently working on clarifying React Native's debugging docs & tooling integrations. We'd like to consider this (or a similar change) holistically as part of that.

gabrieldonadel added a commit to expo/expo that referenced this pull request Apr 6, 2023
# Why

As part of facebook/react-native#36754, we
should remove the ability to use remote JS debugging from the DevMenu.
This change is motivated by the fact that generally speaking, this
feature does not work with the new architecture and most of the popular
modules these days.

Related to ENG-8088

# How

This PR disables the `Remote JS debugger` option inside the dev-menu
when using SDK 49 or above

# Test Plan

Run `dev-menu` locally through bare-expo
 
<table>
    <tr><th>iOS</th><th>Android</th></tr>
    <tr>
    <td>
<img
src="https://user-images.githubusercontent.com/11707729/230195305-5fcf6bbf-f669-40d6-841b-1decc967bebe.png"
height="700px" />
   </td>
   <td>
<img
src="https://user-images.githubusercontent.com/11707729/230218993-b2fd081c-bc03-4330-8a61-29485c512682.png"
height="700px" />
    </td>
</tr> 
</table> 

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg:
updated a module plugin).
gabrieldonadel added a commit to expo/expo that referenced this pull request Apr 10, 2023
# Why

As part of facebook/react-native#36754, we
should remove the ability to use remote JS debugging from the DevMenu.
This change is motivated by the fact that generally speaking, this
feature does not work with the new architecture and most of the popular
modules these days.

Follow up of #22010
Closes ENG-8088

# How

This PR removes the `Remote JS debugger` option from Expo Go menu when
using SDK 49 or above

# Test Plan


Run Expo Go locally 
 
<table>
    <tr><th>iOS</th><th>Android</th></tr>
    <tr>
    <td>
<img
src="https://user-images.githubusercontent.com/11707729/230474710-28b0a90a-9f49-48c1-baaf-8883fdad8178.png"
height="700px" />
   </td>
   <td>
<img
src="https://user-images.githubusercontent.com/11707729/230473058-e06831e6-96d9-4a17-8da1-d884d9c9678f.png"
height="700px" />
    </td>
</tr> 
</table> 
  

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg:
updated a module plugin).
@brentvatne
Copy link
Collaborator

@motiz88 - here is an example of one of the most popular react-native libraries not working with remote JS debugging with JSC: https://github.com/brentvatne/example-jsc-debugging. You can clone that repo and run npx expo run:ios (or npx react-native run-ios if that tickles your fancy), notice that the app works as expected, then once you enable remote JS debugging you won't get much more than red screens and crashes. Many libraries have migrated to using JSI and don't work with remote debugging and the failure mode is very frustrating.

image

I believe developers should use "Direct debugging with Safari" in both new and old architectures if they must use JSC. I confirmed that these instructions work well in the repo shared above.

@motiz88
Copy link
Contributor

motiz88 commented Apr 19, 2023

@brentvatne Given that direct Safari debugging works well¹ in JSC, I'm not in principle opposed to deprecating remote JS debugging. From my perspective we'd need not just this dev menu change, but a docs change and a clear callout in the release announcement where this lands. Also following up to delete the remote debugger code (in a subsequent release?) would be great.

¹ FWIW, per the docs it does look pretty rough around the edges: no source maps out of the box and each reload disconnecting the debugger both sound like a clearly worse experience compared to remote debugging at its best, but I appreciate that that may be moot given the existence of popular libraries that hard-break remote debugging.

@brentvatne
Copy link
Collaborator

@motiz88 - agreed! we'll need to update the documentation for this change and also include it in the release notes. ideally this is cherry-picked in 0.72 - @gabrieldonadel is going to send a docs PR and will follow up with adding information to the release notes as well

@huntie
Copy link
Member

huntie commented Apr 20, 2023

@brentvatne Great! 👋🏻 I'm happy to review PRs and support this effort (currently also looking at our JS Debugging options).

@gabrieldonadel
Copy link
Collaborator Author

@huntie @motiz88 I've just opened a PR (facebook/react-native-website#3702) updating the docs to reflect this change and adding a few more sections about debugging

@kelset
Copy link
Contributor

kelset commented Apr 25, 2023

hey folks - maybe next time involve the release crew if you think of adding something last minute to the release that is in the works.

That said, the Direct debugging with Safari feature at least in the docs is mentioned specifically only for iOS; what about Android developers? Unless it actually works for Android too I don't think at the moment there is one compelling alternative that works out of the box with all platforms (Flipper? and I'm pretty sure we shouldn't be telling folks to double down on Flipper).

Also, since it's Safari bound, what about developers not on macos?

@gabrieldonadel
Copy link
Collaborator Author

Flipper? and I'm pretty sure we shouldn't be telling folks to double down on Flipper?

Why not talk about flipper? The debug button on the dev menu is hard-coded to open it

options.put(
mApplicationContext.getString(R.string.catalyst_debug_open),
new DevOptionHandler() {
@Override
public void onOptionSelected() {
mDevServerHelper.openUrl(
mCurrentContext,
FLIPPER_DEBUGGER_URL,
mApplicationContext.getString(R.string.catalyst_open_flipper_error));
}
});
options.put(
mApplicationContext.getString(R.string.catalyst_devtools_open),
new DevOptionHandler() {
@Override
public void onOptionSelected() {
mDevServerHelper.openUrl(
mCurrentContext,
FLIPPER_DEVTOOLS_URL,
mApplicationContext.getString(R.string.catalyst_open_flipper_error));
}
});

[items addObject:[RCTDevMenuItem
buttonItemWithTitleBlock:^NSString * {
return @"Open Debugger";
}
handler:^{
[RCTInspectorDevServerHelper
openURL:@"flipper://null/Hermesdebuggerrn?device=React%20Native"
withBundleURL:bundleManager.bundleURL
withErrorMessage:@"Failed to open Flipper. Please check that Metro is running."];
}]];
[items addObject:[RCTDevMenuItem
buttonItemWithTitleBlock:^NSString * {
return @"Open React DevTools";
}
handler:^{
[RCTInspectorDevServerHelper
openURL:@"flipper://null/React?device=React%20Native"
withBundleURL:bundleManager.bundleURL
withErrorMessage:@"Failed to open Flipper. Please check that Metro is running."];
}]];

Users can also use debug JS on Hermes using Google Chrome's DevTools as stated here -> https://reactnative.dev/docs/hermes#debugging-js-on-hermes-using-google-chromes-devtools

@kelset
Copy link
Contributor

kelset commented Apr 25, 2023

Why not talk about flipper?

I'll let the Meta folks share more, but the gist is that there's an effort to rethink the entire debugging story and Flipper might not be part of that given how many troubles people have when using it (reactwg/react-native-releases#64). So we should not push people towards something that might not be the right long term option.

Users can also use debug JS on Hermes using Google Chrome's DevTools

That doesn't cover users on JSC or other engines.

Echoing what @huntie said in the other PR (facebook/react-native-website#3702 (review)) I think it's better to think of this effort as something for 0.73 and onward, to give time to come up with a solution that will allow all developers to use something viable.

Copy link
Member

@huntie huntie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to approve this, but let's target React Native 0.73 (regular merge).

As @kelset noted, the React Native team is holistically reviewing our JS debugging options under the New Architecture and our timeline is for React Native 0.73. This work closely overlaps with this PR, and we believe we can serve users best if we batch these changes (and don't ship a major workflow change for users at this point in the 0.72 RC). (We aim to share more info on our plans later this week.)

Note: Ideally, we'd introduce a replacement flow for JSC debugging on Android before removing this capability. However, given remote JSC debugging is generally broken out-of-the-box today, and users can opt to re-enable it, I think this is fine.


(As commented on other PR) I'm happy to document Flipper for the time being, as it's been the default JS debugging flow for a while and the updates accurately describe the current state.

@facebook-github-bot
Copy link
Contributor

@huntie has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Apr 26, 2023
@facebook-github-bot
Copy link
Contributor

@huntie merged this pull request in 28e1ca9.

@gabrieldonadel gabrieldonadel deleted the remove-remove-debugging branch April 26, 2023 17:13
jeongshin pushed a commit to jeongshin/react-native that referenced this pull request May 7, 2023
Summary:
This PR proposes the removal of the ability to use remote JS debugging (a.k.a Chrome Debugging) from the DevMenu.
This change has been suggested previously on the contributor's Discord server and it is motivated by the fact that generally speaking, this feature does not work with the new architecture and most of the popular modules these days. The remote JS debugging is basically broken if you use Turbo Modules, Fabric, or sync native module calls.

People tend to assume that if something is part of the dev tools UI, it is going to work reliably. However, when it comes to remote debugging using JSC that's increasingly unlikely to be the case. Having it continue to exist as an option has created some confusion, or at least that's what we've seen within the Expo community.

### How to manually enable remote debugging

If your project depends on remote debugging, you can still enable it manually through the `NativeDevSettings`. E.g.

```jsx
import NativeDevSettings from 'react-native/Libraries/NativeModules/specs/NativeDevSettings';

export default function App() {
  return (
    <Button
      title="Enable remote debugging"
      onPress={() => NativeDevSettings.setIsDebuggingRemotely(!true)}
    />
  );
}

```

### Next steps

Once this has been released and we are sure that this doesn't cause a lot of problems for people we can remove the ability entirely in a follow-up PR

## Changelog:

[General] [Removed] - Remove remote debugging from the dev menu

Pull Request resolved: facebook#36754

Test Plan:
Locally run rn-tester using JSC on Android and iOS and open the dev menu

<table>
    <tr><th>iOS</th><th>Android</th></tr>
    <tr>
    <td>
        <img src="https://user-images.githubusercontent.com/11707729/229229079-03f98eed-0765-4cc8-b972-0c4ff041b611.png" />
   </td>
   <td>
   <img src="https://user-images.githubusercontent.com/11707729/229229103-b57e8fd2-9aca-4780-8a8f-0fd5b2e53590.png" />
    </td>
</tr>
</table>

Reviewed By: christophpurrer

Differential Revision: D45278061

Pulled By: huntie

fbshipit-source-id: 842c33102cd34730c14acece8e318cb263e5c9e5
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
This PR proposes the removal of the ability to use remote JS debugging (a.k.a Chrome Debugging) from the DevMenu.
This change has been suggested previously on the contributor's Discord server and it is motivated by the fact that generally speaking, this feature does not work with the new architecture and most of the popular modules these days. The remote JS debugging is basically broken if you use Turbo Modules, Fabric, or sync native module calls.

People tend to assume that if something is part of the dev tools UI, it is going to work reliably. However, when it comes to remote debugging using JSC that's increasingly unlikely to be the case. Having it continue to exist as an option has created some confusion, or at least that's what we've seen within the Expo community.

### How to manually enable remote debugging

If your project depends on remote debugging, you can still enable it manually through the `NativeDevSettings`. E.g.

```jsx
import NativeDevSettings from 'react-native/Libraries/NativeModules/specs/NativeDevSettings';

export default function App() {
  return (
    <Button
      title="Enable remote debugging"
      onPress={() => NativeDevSettings.setIsDebuggingRemotely(!true)}
    />
  );
}

```

### Next steps

Once this has been released and we are sure that this doesn't cause a lot of problems for people we can remove the ability entirely in a follow-up PR

## Changelog:

[General] [Removed] - Remove remote debugging from the dev menu

Pull Request resolved: facebook#36754

Test Plan:
Locally run rn-tester using JSC on Android and iOS and open the dev menu

<table>
    <tr><th>iOS</th><th>Android</th></tr>
    <tr>
    <td>
        <img src="https://user-images.githubusercontent.com/11707729/229229079-03f98eed-0765-4cc8-b972-0c4ff041b611.png" />
   </td>
   <td>
   <img src="https://user-images.githubusercontent.com/11707729/229229103-b57e8fd2-9aca-4780-8a8f-0fd5b2e53590.png" />
    </td>
</tr>
</table>

Reviewed By: christophpurrer

Differential Revision: D45278061

Pulled By: huntie

fbshipit-source-id: 842c33102cd34730c14acece8e318cb263e5c9e5
huntie added a commit to huntie/react-native that referenced this pull request Oct 19, 2023
Summary:
## Context

**Remote JS Debugging removal**

In React Native 0.73, we have deprecated Remote JS Debugging (execution of JavaScript in a separate V8 process) and also removed the Dev Menu launcher (facebook#36754).

This was motivated primarily by the broken state of this feature — in particular: incompatibility with the New Architecture. Secondly, as the React Native team continues to invest in improving debugging, we want to reduce the surface area of debugging modes that we support.

During this year, we reached out to our partners about the impact of removing this feature, and if there was interest in having separate community support for debugging JSC on Android (a limited use case affected by this change). Without strong interest, we concluded that dropping Remote JS Debugging support in core for React Native 0.74 makes sense. We are focusing future debugging efforts into providing a first-class experience for Hermes.

Existing alternatives:

- [Direct debugging for JSC (Safari, iOS only)](https://reactnative.dev/docs/0.71/debugging#safari-developer-tools).
- Direct debugging with Hermes: Flipper, [Chrome (old method)](https://reactnative.dev/docs/0.71/hermes#debugging-js-on-hermes-using-google-chromes-devtools), or via `react-native start --experimental-debugger` (intended future default).

## This diff

This removes remaininng Remote JS Debugging code and `RCTWebSocketExecutor` on iOS.

Changelog: [iOS][Breaking] Remove remote JS debugging capability (JSC projects)

Differential Revision: D50326296
huntie added a commit to huntie/react-native that referenced this pull request Oct 19, 2023
Summary:
## Context

**Remote JS Debugging removal**

In React Native 0.73, we have deprecated Remote JS Debugging (execution of JavaScript in a separate V8 process) and also removed the Dev Menu launcher (facebook#36754).

This was motivated primarily by the broken state of this feature — in particular: incompatibility with the New Architecture. Secondly, as the React Native team continues to invest in improving debugging, we want to reduce the surface area of debugging modes that we support.

During this year, we reached out to our partners about the impact of removing this feature, and if there was interest in having separate community support for debugging JSC on Android (a limited use case affected by this change). Without strong interest, we concluded that dropping Remote JS Debugging support in core for React Native 0.74 makes sense. We are focusing future debugging efforts into providing a first-class experience for Hermes.

Existing alternatives:

- [Direct debugging for JSC (Safari, iOS only)](https://reactnative.dev/docs/0.71/debugging#safari-developer-tools).
- Direct debugging with Hermes: Flipper, [Chrome (old method)](https://reactnative.dev/docs/0.71/hermes#debugging-js-on-hermes-using-google-chromes-devtools), or via `react-native start --experimental-debugger` (intended future default).

## This diff

This removes remaining Remote JS Debugging code on Android.

Changelog: [Android][Breaking] Remove remote JS debugging capability (JSC projects)

Differential Revision: D50325682
huntie added a commit to huntie/react-native that referenced this pull request Nov 17, 2023
Summary:
## Context

**Remote JS Debugging removal**

In React Native 0.73, we have deprecated Remote JS Debugging (execution of JavaScript in a separate V8 process) and also removed the Dev Menu launcher (facebook#36754).

## This diff

Follows D46187942 — this option wasn't correctly removed for Android when running JSC. This is now consistent with iOS.

Changelog:
[Android][Changed] "Open Debugger" is no longer available for remote JS debugging from the Dev Menu (non-Hermes). Please use `NativeDevSettings.setIsDebuggingRemotely()`.

Differential Revision: D50555095
huntie added a commit to huntie/react-native that referenced this pull request Nov 17, 2023
Summary:

## Context

**Remote JS Debugging removal**

In React Native 0.73, we have deprecated Remote JS Debugging (execution of JavaScript in a separate V8 process) and also removed the Dev Menu launcher (facebook#36754).

## This diff

Follows D46187942 — this option wasn't correctly removed for Android when running JSC. This is now consistent with iOS.

Changelog:
[Android][Changed] "Open Debugger" is no longer available for remote JS debugging from the Dev Menu (non-Hermes). Please use `NativeDevSettings.setIsDebuggingRemotely()`.

Reviewed By: blakef

Differential Revision: D50555095
facebook-github-bot pushed a commit that referenced this pull request Nov 17, 2023
Summary:
Pull Request resolved: #41535

## Context

**Remote JS Debugging removal**

In React Native 0.73, we have deprecated Remote JS Debugging (execution of JavaScript in a separate V8 process) and also removed the Dev Menu launcher (#36754).

## This diff

Follows D46187942 — this option wasn't correctly removed for Android when running JSC. This is now consistent with iOS.

Changelog:
[Android][Changed] "Open Debugger" is no longer available for remote JS debugging from the Dev Menu (non-Hermes). Please use `NativeDevSettings.setIsDebuggingRemotely()`.

Reviewed By: blakef

Differential Revision: D50555095

fbshipit-source-id: 1aeb48ab1390dc12ce300d6f321c30de5343cf0a
huntie added a commit that referenced this pull request Nov 21, 2023
Summary:
Pull Request resolved: #41535

## Context

**Remote JS Debugging removal**

In React Native 0.73, we have deprecated Remote JS Debugging (execution of JavaScript in a separate V8 process) and also removed the Dev Menu launcher (#36754).

## This diff

Follows D46187942 — this option wasn't correctly removed for Android when running JSC. This is now consistent with iOS.

Changelog:
[Android][Changed] "Open Debugger" is no longer available for remote JS debugging from the Dev Menu (non-Hermes). Please use `NativeDevSettings.setIsDebuggingRemotely()`.

Reviewed By: blakef

Differential Revision: D50555095

fbshipit-source-id: 1aeb48ab1390dc12ce300d6f321c30de5343cf0a
Othinn pushed a commit to Othinn/react-native that referenced this pull request Jan 9, 2024
Summary:
Pull Request resolved: facebook#41535

## Context

**Remote JS Debugging removal**

In React Native 0.73, we have deprecated Remote JS Debugging (execution of JavaScript in a separate V8 process) and also removed the Dev Menu launcher (facebook#36754).

## This diff

Follows D46187942 — this option wasn't correctly removed for Android when running JSC. This is now consistent with iOS.

Changelog:
[Android][Changed] "Open Debugger" is no longer available for remote JS debugging from the Dev Menu (non-Hermes). Please use `NativeDevSettings.setIsDebuggingRemotely()`.

Reviewed By: blakef

Differential Revision: D50555095

fbshipit-source-id: 1aeb48ab1390dc12ce300d6f321c30de5343cf0a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Expo Partner: Expo Partner Type: Removal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants