Unify codepath for retrieveing metro location on Android (#42617)

Summary:
With the current ways metro location is determined, when we want to use a different metro port this requires app to be rebuild as the port and location are stored in resource file that gets compiled to R.class. The only way to avoid app rebuild due to a port change is to use shared preferences that can be accessed from dev menu, where metro URL can be specified. However, due to a separate code-paths for retrieving bundle location and for `/inspector/device` calls, the setting only applies to the former. As a consequence, you can change metro URL in the shared preferences, but debugging would only work if you use the default port or you rebuild the app with the correct port number.

This PR removes the separate code-path for retrieving inspector URL including all the dependencies scattered across different files including the gradle plugin. We then replace calls to `PackagerConnectionSettings.getInspectorServerHost` with `PackagerConnectionSettings.getDebugServerHost` which respects the shared preferences and other possible ways of configuring the port.

I decided to remove the separate inspector URL code path, as the resource value for inspector port added in https://github.com/facebook/react-native/issues/23616 was never functioning properly due to a bug. In the said PR introduced a bug in [AndroidInfoHelpers.java](https://github.com/facebook/react-native/blob/a13d51ff1c38ea85e59f4215563c0dd05452f670/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/systeminfo/AndroidInfoHelpers.java#L77) where `react_native_dev_server_port` was used instead `react_native_inspector_proxy_port`. As a result the added resource value was never read.

This can be potentially a breaking change as I'm removing some public methods. However I think it is unlikely anyone relied on said methods. As a part of this PR I'm also changing occurences of removed methods from ReactAndroid.api – I don't know how to test those changes since I don't understand how this file is used as it doesn't have any references in public code.

## Changelog:

[ANDROID] [FIXED] - Make Android respect metro location from shared preferences for the debugger workflow

Pull Request resolved: https://github.com/facebook/react-native/pull/42617

Test Plan:
1. Run android app on emulator using default port
2. Check the debugger works when using "Open Debugger" option from dev menu
3. Restart metro with custom port (`--port 9090`) while keeping the app running
4. Open dev menu, click "Settings" then "Debug server host & port", put "10.0.2.2:9090" there
5. Reload the app
6. Before this change things like hot reload would continue to work while "Open Debugger" option would do nothing
7. After this change both reloading and debugging will work

Important: I haven't tested changes made to ReactAndroid.api as I don't know what this files is used for with no references in the codebase.

Reviewed By: cortinico

Differential Revision: D53010023

Pulled By: huntie

fbshipit-source-id: cc8b9c5c7e834ec9ea02b1ed5acf94f04f7b7116
This commit is contained in:
Krzysztof Magiera
2024-01-25 02:29:13 -08:00
committed by Facebook GitHub Bot
parent 631b6a12c5
commit 0ea16fd3f7
6 changed files with 2 additions and 27 deletions
@@ -53,15 +53,11 @@ internal object AgpConfiguratorUtils {
fun configureDevPorts(project: Project) {
val devServerPort =
project.properties["reactNativeDevServerPort"]?.toString() ?: DEFAULT_DEV_SERVER_PORT
val inspectorProxyPort =
project.properties["reactNativeInspectorProxyPort"]?.toString() ?: devServerPort
val action =
Action<AppliedPlugin> {
project.extensions.getByType(AndroidComponentsExtension::class.java).finalizeDsl { ext ->
ext.defaultConfig.resValue("integer", "react_native_dev_server_port", devServerPort)
ext.defaultConfig.resValue(
"integer", "react_native_inspector_proxy_port", inspectorProxyPort)
}
}
@@ -3452,7 +3452,6 @@ public class com/facebook/react/modules/systeminfo/AndroidInfoHelpers {
public static fun getAdbReverseTcpCommand (Landroid/content/Context;)Ljava/lang/String;
public static fun getAdbReverseTcpCommand (Ljava/lang/Integer;)Ljava/lang/String;
public static fun getFriendlyDeviceName ()Ljava/lang/String;
public static fun getInspectorProxyHost (Landroid/content/Context;)Ljava/lang/String;
public static fun getServerHost (Landroid/content/Context;)Ljava/lang/String;
public static fun getServerHost (Ljava/lang/Integer;)Ljava/lang/String;
}
@@ -3528,7 +3527,6 @@ public abstract class com/facebook/react/packagerconnection/NotificationOnlyHand
public class com/facebook/react/packagerconnection/PackagerConnectionSettings {
public fun <init> (Landroid/content/Context;)V
public fun getDebugServerHost ()Ljava/lang/String;
public fun getInspectorServerHost ()Ljava/lang/String;
public fun getPackageName ()Ljava/lang/String;
public fun setDebugServerHost (Ljava/lang/String;)V
}
@@ -429,11 +429,6 @@ fun reactNativeDevServerPort(): String {
return value?.toString() ?: "8081"
}
fun reactNativeInspectorProxyPort(): String {
val value = project.properties["reactNativeInspectorProxyPort"]
return value?.toString() ?: reactNativeDevServerPort()
}
fun reactNativeArchitectures(): List<String> {
val value = project.properties["reactNativeArchitectures"]
return value?.toString()?.split(",") ?: listOf("armeabi-v7a", "x86", "x86_64", "arm64-v8a")
@@ -498,7 +493,6 @@ android {
buildConfigField("int", "EXOPACKAGE_FLAGS", "0")
resValue("integer", "react_native_dev_server_port", reactNativeDevServerPort())
resValue("integer", "react_native_inspector_proxy_port", reactNativeInspectorProxyPort())
testApplicationId = "com.facebook.react.tests.gradle"
testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
@@ -312,7 +312,7 @@ public class DevServerHelper {
return String.format(
Locale.US,
"http://%s/inspector/device?name=%s&app=%s&device=%s",
mPackagerConnectionSettings.getInspectorServerHost(),
mPackagerConnectionSettings.getDebugServerHost(),
Uri.encode(AndroidInfoHelpers.getFriendlyDeviceName()),
Uri.encode(mPackageName),
Uri.encode(getInspectorDeviceId()));
@@ -491,7 +491,7 @@ public class DevServerHelper {
String.format(
Locale.US,
"http://%s/open-debugger?appId=%s&device=%s",
mPackagerConnectionSettings.getInspectorServerHost(),
mPackagerConnectionSettings.getDebugServerHost(),
Uri.encode(mPackageName),
Uri.encode(getInspectorDeviceId()));
Request request =
@@ -52,10 +52,6 @@ public class AndroidInfoHelpers {
return getAdbReverseTcpCommand(getDevServerPort(context));
}
public static String getInspectorProxyHost(Context context) {
return getServerIpAddress(getInspectorProxyPort(context));
}
// WARNING(festevezga): This RN helper method has been copied to another FB-only target. Any
// changes should be applied to both.
public static String getFriendlyDeviceName() {
@@ -72,11 +68,6 @@ public class AndroidInfoHelpers {
return resources.getInteger(R.integer.react_native_dev_server_port);
}
private static Integer getInspectorProxyPort(Context context) {
Resources resources = context.getResources();
return resources.getInteger(R.integer.react_native_dev_server_port);
}
private static String getServerIpAddress(int port) {
// Since genymotion runs in vbox it use different hostname to refer to adb host.
// We detect whether app runs on genymotion and replace js bundle server hostname accordingly
@@ -57,10 +57,6 @@ public class PackagerConnectionSettings {
mPreferences.edit().putString(PREFS_DEBUG_SERVER_HOST_KEY, host).apply();
}
public String getInspectorServerHost() {
return AndroidInfoHelpers.getInspectorProxyHost(mAppContext);
}
public @Nullable String getPackageName() {
return mPackageName;
}