Skip to content

Commit

Permalink
Remove remote JS debugging (Android)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
huntie authored and facebook-github-bot committed Oct 19, 2023
1 parent 4134d8d commit 9907735
Show file tree
Hide file tree
Showing 13 changed files with 6 additions and 248 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@ public boolean isDeviceDebugEnabled() {
return false;
}

@Override
public boolean isRemoteJSDebugEnabled() {
return false;
}

@Override
public void setRemoteJSDebugEnabled(boolean remoteJSDebugEnabled) {}

@Override
public boolean isStartSamplingProfilerOnInit() {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,10 @@
import com.facebook.react.bridge.JSExceptionHandler;
import com.facebook.react.bridge.JSIModulePackage;
import com.facebook.react.bridge.JSIModuleType;
import com.facebook.react.bridge.JavaJSExecutor;
import com.facebook.react.bridge.JavaScriptExecutor;
import com.facebook.react.bridge.JavaScriptExecutorFactory;
import com.facebook.react.bridge.NativeModuleRegistry;
import com.facebook.react.bridge.NotThreadSafeBridgeIdleDebugListener;
import com.facebook.react.bridge.ProxyJavaScriptExecutor;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.ReactCxxErrorHandler;
Expand Down Expand Up @@ -297,11 +295,6 @@ public static ReactInstanceManagerBuilder builder() {

private ReactInstanceDevHelper createDevHelperInterface() {
return new ReactInstanceDevHelper() {
@Override
public void onReloadWithJSDebugger(JavaJSExecutor.Factory jsExecutorFactory) {
ReactInstanceManager.this.onReloadWithJSDebugger(jsExecutorFactory);
}

@Override
public void onJSBundleLoadedFromServer() {
ReactInstanceManager.this.onJSBundleLoadedFromServer();
Expand Down Expand Up @@ -448,14 +441,9 @@ public void onPackagerStatusFetched(final boolean packagerIsRunning) {
if (packagerIsRunning) {
mDevSupportManager.handleReloadJS();
} else if (mDevSupportManager.hasUpToDateJSBundleInCache()
&& !devSettings.isRemoteJSDebugEnabled()
&& !mUseFallbackBundle) {
// If there is a up-to-date bundle downloaded from server,
// with remote JS debugging disabled, always use that.
onJSBundleLoadedFromServer();
} else {
// If dev server is down, disable the remote JS debugging.
devSettings.setRemoteJSDebugEnabled(false);
recreateReactContextInBackgroundFromBundleLoader();
}
});
Expand Down Expand Up @@ -1023,16 +1011,6 @@ public String getJsExecutorName() {
return mJavaScriptExecutorFactory.toString();
}

@ThreadConfined(UI)
private void onReloadWithJSDebugger(JavaJSExecutor.Factory jsExecutorFactory) {
FLog.d(ReactConstants.TAG, "ReactInstanceManager.onReloadWithJSDebugger()");
recreateReactContextInBackground(
new ProxyJavaScriptExecutor.Factory(jsExecutorFactory),
JSBundleLoader.createRemoteDebuggerBundleLoader(
mDevSupportManager.getJSBundleURLForRemoteDebugging(),
mDevSupportManager.getSourceUrl()));
}

@ThreadConfined(UI)
private void onJSBundleLoadedFromServer() {
FLog.d(ReactConstants.TAG, "ReactInstanceManager.onJSBundleLoadedFromServer()");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,6 @@ public String loadScript(JSBundleLoaderDelegate delegate) {
};
}

/**
* This loader is used when proxy debugging is enabled. In that case there is no point in fetching
* the bundle from device as remote executor will have to do it anyway.
*/
public static JSBundleLoader createRemoteDebuggerBundleLoader(
final String proxySourceURL, final String realSourceURL) {
return new JSBundleLoader() {
@Override
public String loadScript(JSBundleLoaderDelegate delegate) {
delegate.setSourceURLs(realSourceURL, proxySourceURL);
return realSourceURL;
}
};
}

/** Loads the script, returning the URL of the source it loaded. */
public abstract String loadScript(JSBundleLoaderDelegate delegate);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.CatalystInstance;
import com.facebook.react.bridge.JSBundleLoader;
import com.facebook.react.bridge.JavaJSExecutor;
import com.facebook.react.bridge.JavaScriptExecutorFactory;
import com.facebook.react.bridge.ReactMarker;
import com.facebook.react.bridge.ReactMarkerConstants;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.common.SurfaceDelegateFactory;
import com.facebook.react.common.futures.SimpleSettableFuture;
import com.facebook.react.devsupport.interfaces.DevBundleDownloadListener;
import com.facebook.react.devsupport.interfaces.DevLoadingViewManager;
import com.facebook.react.devsupport.interfaces.DevOptionHandler;
Expand All @@ -33,9 +31,6 @@
import java.io.File;
import java.io.IOException;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

/**
* Interface for accessing and interacting with development features. Following features
Expand Down Expand Up @@ -136,54 +131,6 @@ public void onError(String url, Throwable cause) {
});
}

private WebsocketJavaScriptExecutor.JSExecutorConnectCallback getExecutorConnectCallback(
final SimpleSettableFuture<Boolean> future) {
return new WebsocketJavaScriptExecutor.JSExecutorConnectCallback() {
@Override
public void onSuccess() {
future.set(true);
hideDevLoadingView();
}

@Override
public void onFailure(final Throwable cause) {
hideDevLoadingView();
FLog.e(ReactConstants.TAG, "Failed to connect to debugger!", cause);
future.setException(
new IOException(
getApplicationContext().getString(com.facebook.react.R.string.catalyst_debug_error),
cause));
}
};
}

private void reloadJSInProxyMode() {
// When using js proxy, there is no need to fetch JS bundle as proxy executor will do that
// anyway
getDevServerHelper().launchJSDevtools();

JavaJSExecutor.Factory factory =
new JavaJSExecutor.Factory() {
@Override
public JavaJSExecutor create() throws Exception {
WebsocketJavaScriptExecutor executor = new WebsocketJavaScriptExecutor();
SimpleSettableFuture<Boolean> future = new SimpleSettableFuture<>();
executor.connect(
getDevServerHelper().getWebsocketProxyURL(), getExecutorConnectCallback(future));
// TODO(t9349129) Don't use timeout
try {
future.get(90, TimeUnit.SECONDS);
return executor;
} catch (ExecutionException e) {
throw (Exception) e.getCause();
} catch (InterruptedException | TimeoutException e) {
throw new RuntimeException(e);
}
}
};
getReactInstanceDevHelper().onReloadWithJSDebugger(factory);
}

@Override
public void handleReloadJS() {

Expand All @@ -196,19 +143,11 @@ public void handleReloadJS() {
// dismiss redbox if exists
hideRedboxDialog();

if (getDevSettings().isRemoteJSDebugEnabled()) {
PrinterHolder.getPrinter()
.logMessage(ReactDebugOverlayTags.RN_CORE, "RNCore: load from Proxy");
showDevLoadingViewForRemoteJSEnabled();
reloadJSInProxyMode();
} else {
PrinterHolder.getPrinter()
.logMessage(ReactDebugOverlayTags.RN_CORE, "RNCore: load from Server");
String bundleURL =
getDevServerHelper()
.getDevServerBundleURL(Assertions.assertNotNull(getJSAppBundleName()));
reloadJSFromServer(bundleURL);
}
PrinterHolder.getPrinter()
.logMessage(ReactDebugOverlayTags.RN_CORE, "RNCore: load from Server");
String bundleURL =
getDevServerHelper().getDevServerBundleURL(Assertions.assertNotNull(getJSAppBundleName()));
reloadJSFromServer(bundleURL);
}

/** Starts of stops the sampling profiler */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ class DevInternalSettings
private static final String PREFS_ANIMATIONS_DEBUG_KEY = "animations_debug";
private static final String PREFS_INSPECTOR_DEBUG_KEY = "inspector_debug";
private static final String PREFS_HOT_MODULE_REPLACEMENT_KEY = "hot_module_replacement";
private static final String PREFS_REMOTE_JS_DEBUG_KEY = "remote_js_debug";
private static final String PREFS_START_SAMPLING_PROFILER_ON_INIT =
"start_sampling_profiler_on_init";

Expand Down Expand Up @@ -85,16 +84,6 @@ public boolean isDeviceDebugEnabled() {
return ReactBuildConfig.DEBUG;
}

@Override
public boolean isRemoteJSDebugEnabled() {
return mPreferences.getBoolean(PREFS_REMOTE_JS_DEBUG_KEY, false);
}

@Override
public void setRemoteJSDebugEnabled(boolean remoteJSDebugEnabled) {
mPreferences.edit().putBoolean(PREFS_REMOTE_JS_DEBUG_KEY, remoteJSDebugEnabled).apply();
}

@Override
public boolean isStartSamplingProfilerOnInit() {
return mPreferences.getBoolean(PREFS_START_SAMPLING_PROFILER_ON_INIT, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.facebook.common.logging.FLog;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.devsupport.interfaces.DevBundleDownloadListener;
Expand Down Expand Up @@ -56,8 +55,6 @@
* </ul>
*/
public class DevServerHelper {
public static final String RELOAD_APP_EXTRA_JS_PROXY = "jsproxy";

private static final int HTTP_CONNECT_TIMEOUT_MS = 5000;

private static final String DEBUGGER_MSG_DISABLE = "{ \"id\":1,\"method\":\"Debugger.disable\" }";
Expand Down Expand Up @@ -237,13 +234,6 @@ protected Void doInBackground(Void... params) {
}.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
}

public String getWebsocketProxyURL() {
return String.format(
Locale.US,
"ws://%s/debugger-proxy?role=client",
mPackagerConnectionSettings.getDebugServerHost());
}

private String getInspectorDeviceUrl() {
return String.format(
Locale.US,
Expand All @@ -261,28 +251,6 @@ public void downloadBundleFromURL(
mBundleDownloader.downloadBundleFromURL(callback, outputFile, bundleURL, bundleInfo);
}

public void downloadBundleFromURL(
DevBundleDownloadListener callback,
File outputFile,
String bundleURL,
BundleDownloader.BundleInfo bundleInfo,
Request.Builder requestBuilder) {
mBundleDownloader.downloadBundleFromURL(
callback, outputFile, bundleURL, bundleInfo, requestBuilder);
}

/** @return the host to use when connecting to the bundle server from the host itself. */
private String getHostForJSProxy() {
// Use custom port if configured. Note that host stays "localhost".
String host = Assertions.assertNotNull(mPackagerConnectionSettings.getDebugServerHost());
int portOffset = host.lastIndexOf(':');
if (portOffset > -1) {
return "localhost" + host.substring(portOffset);
} else {
return AndroidInfoHelpers.DEVICE_LOCALHOST;
}
}

/** @return whether we should enable dev mode when requesting JS bundles. */
private boolean getDevMode() {
return mSettings.isJSDevModeEnabled();
Expand Down Expand Up @@ -345,32 +313,6 @@ public void isPackagerRunning(final PackagerStatusCallback callback) {
}
}

private String createLaunchJSDevtoolsCommandUrl() {
return String.format(
Locale.US,
"http://%s/launch-js-devtools",
mPackagerConnectionSettings.getDebugServerHost());
}

public void launchJSDevtools() {
Request request = new Request.Builder().url(createLaunchJSDevtoolsCommandUrl()).build();
mClient
.newCall(request)
.enqueue(
new Callback() {
@Override
public void onFailure(@NonNull Call call, @NonNull IOException e) {
// ignore HTTP call response, this is just to open a debugger page and there is no
// reason to report failures from here
}

@Override
public void onResponse(@NonNull Call call, @NonNull Response response) {
// ignore HTTP call response - see above
}
});
}

public String getSourceMapUrl(String mainModuleName) {
return createBundleURL(mainModuleName, BundleType.MAP);
}
Expand All @@ -379,13 +321,6 @@ public String getSourceUrl(String mainModuleName) {
return createBundleURL(mainModuleName, BundleType.BUNDLE);
}

public String getJSBundleURLForRemoteDebugging(String mainModuleName) {
// The host we use when connecting to the JS bundle server from the emulator is not the
// same as the one needed to connect to the same server from the JavaScript proxy running on the
// host itself.
return createBundleURL(mainModuleName, BundleType.BUNDLE, getHostForJSProxy());
}

/**
* This is a debug-only utility to allow fetching a file via packager. It's made synchronous for
* simplicity, but should only be used if it's absolutely necessary.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,6 @@ public DevSupportManagerBase(
public void onReceive(Context context, Intent intent) {
String action = intent.getAction();
if (getReloadAppAction(context).equals(action)) {
if (intent.getBooleanExtra(DevServerHelper.RELOAD_APP_EXTRA_JS_PROXY, false)) {
mDevSettings.setRemoteJSDebugEnabled(true);
mDevServerHelper.launchJSDevtools();
} else {
mDevSettings.setRemoteJSDebugEnabled(false);
}
handleReloadJS();
}
}
Expand Down Expand Up @@ -367,13 +361,6 @@ public void onOptionSelected() {

if (mDevSettings.isDeviceDebugEnabled()) {
// On-device JS debugging (CDP). Render action to open debugger frontend.

// Reset the old debugger setting so no one gets stuck.
// TODO: Remove in a few weeks.
if (mDevSettings.isRemoteJSDebugEnabled()) {
mDevSettings.setRemoteJSDebugEnabled(false);
handleReloadJS();
}
options.put(
mApplicationContext.getString(R.string.catalyst_debug_open),
() ->
Expand Down Expand Up @@ -595,12 +582,6 @@ public String getSourceUrl() {
return mDevServerHelper.getSourceUrl(Assertions.assertNotNull(mJSAppBundleName));
}

@Override
public String getJSBundleURLForRemoteDebugging() {
return mDevServerHelper.getJSBundleURLForRemoteDebugging(
Assertions.assertNotNull(mJSAppBundleName));
}

@Override
public String getDownloadedJSBundleFile() {
return mJSBundleDownloadedFile.getAbsolutePath();
Expand Down Expand Up @@ -966,19 +947,6 @@ public void setHotModuleReplacementEnabled(final boolean isHotModuleReplacementE
});
}

@Override
public void setRemoteJSDebugEnabled(final boolean isRemoteJSDebugEnabled) {
if (!mIsDevSupportEnabled) {
return;
}

UiThreadUtil.runOnUiThread(
() -> {
mDevSettings.setRemoteJSDebugEnabled(isRemoteJSDebugEnabled);
handleReloadJS();
});
}

@Override
public void setFpsDebugEnabled(final boolean isFpsDebugEnabled) {
if (!mIsDevSupportEnabled) {
Expand Down
Loading

0 comments on commit 9907735

Please sign in to comment.