Skip to content

Commit

Permalink
Android Fix for 9145: No longer hard code build port (facebook#23616)
Browse files Browse the repository at this point in the history
Summary:
### Problem

According to facebook#9145, the `--port` setting is not respected when executing `react-native run-android`. The templates that report things like what port the dev server runs on are hard coded as well.

### Solution

This commit replaces the hardcoded instances of port 8081 on Android with a build configuration property. This allows setting of the port React Native Android connects to for the local build server.

For this change to work, there must also be an update to the react native CLI to pass along this setting:

react-native-community/cli@master...nhunzaker:9145-android-no-port-hardcode-cli

To avoid some noise on their end, I figured I wouldn't submit a PR until it's this approach is deemed workable.

## Changelog

[Android][fixed] - `react-native run-android --port <x>` correctly connects to dev server and related error messages display the correct port
Pull Request resolved: facebook#23616

Differential Revision: D15645200

Pulled By: cpojer

fbshipit-source-id: 3bdfd458b8ac3ec78290736c9ed0db2e5776ed46
  • Loading branch information
nhunzaker authored and M-i-k-e-l committed Mar 10, 2020
1 parent d308ba0 commit df3ec68
Show file tree
Hide file tree
Showing 12 changed files with 119 additions and 32 deletions.
14 changes: 14 additions & 0 deletions ReactAndroid/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,16 @@ def findNdkBuildFullPath() {
return null
}

def reactNativeDevServerPort() {
def value = project.getProperties().get("reactNativeDevServerPort")
return value != null ? value : "8081"
}

def reactNativeInspectorProxyPort() {
def value = project.getProperties().get("reactNativeInspectorProxyPort")
return value != null ? value : reactNativeDevServerPort()
}

def getNdkBuildFullPath() {
def ndkBuildFullPath = findNdkBuildFullPath()
if (ndkBuildFullPath == null) {
Expand Down Expand Up @@ -288,6 +298,10 @@ android {

buildConfigField("boolean", "IS_INTERNAL_BUILD", "false")
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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public String loadScript(JSBundleLoaderDelegate delegate) {
delegate.loadScriptFromFile(cachedFileLocation, sourceURL, false);
return sourceURL;
} catch (Exception e) {
throw DebugServerException.makeGeneric(e.getMessage(), e);
throw DebugServerException.makeGeneric(sourceURL, e.getMessage(), e);
}
}
};
Expand All @@ -94,7 +94,7 @@ public String loadScript(JSBundleLoaderDelegate delegate) {
delegate.loadScriptFromDeltaBundle(sourceURL, nativeDeltaClient, false);
return sourceURL;
} catch (Exception e) {
throw DebugServerException.makeGeneric(e.getMessage(), e);
throw DebugServerException.makeGeneric(sourceURL, e.getMessage(), e);
}
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import java.io.IOException;

import android.net.Uri;
import android.text.TextUtils;

import com.facebook.common.logging.FLog;
Expand All @@ -28,15 +29,19 @@ public class DebugServerException extends RuntimeException {
"\u2022 Ensure that the packager server is running\n" +
"\u2022 Ensure that your device/emulator is connected to your machine and has USB debugging enabled - run 'adb devices' to see a list of connected devices\n" +
"\u2022 Ensure Airplane Mode is disabled\n" +
"\u2022 If you're on a physical device connected to the same machine, run 'adb reverse tcp:8081 tcp:8081' to forward requests from your device\n" +
"\u2022 If your device is on the same Wi-Fi network, set 'Debug server host & port for device' in 'Dev settings' to your machine's IP address and the port of the local dev server - e.g. 10.0.1.1:8081\n\n";
"\u2022 If you're on a physical device connected to the same machine, run 'adb reverse tcp:<PORT> tcp:<PORT>' to forward requests from your device\n" +
"\u2022 If your device is on the same Wi-Fi network, set 'Debug server host & port for device' in 'Dev settings' to your machine's IP address and the port of the local dev server - e.g. 10.0.1.1:<PORT>\n\n";

public static DebugServerException makeGeneric(String reason, Throwable t) {
return makeGeneric(reason, "", t);
public static DebugServerException makeGeneric(String url, String reason, Throwable t) {
return makeGeneric(url, reason, "", t);
}

public static DebugServerException makeGeneric(String reason, String extra, Throwable t) {
return new DebugServerException(reason + GENERIC_ERROR_MESSAGE + extra, t);
public static DebugServerException makeGeneric(String url, String reason, String extra, Throwable t) {
Uri uri = Uri.parse(url);

String message = GENERIC_ERROR_MESSAGE.replace("<PORT>", String.valueOf(uri.getPort()));

return new DebugServerException(reason + message + extra, t);
}

private DebugServerException(String description, String fileName, int lineNumber, int column) {
Expand All @@ -56,7 +61,7 @@ public DebugServerException(String detailMessage, Throwable throwable) {
* @param str json string returned by the debug server
* @return A DebugServerException or null if the string is not of proper form.
*/
@Nullable public static DebugServerException parse(String str) {
@Nullable public static DebugServerException parse(String url, String str) {
if (TextUtils.isEmpty(str)) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,12 @@ public void onFailure(Call call, IOException e) {
}
mDownloadBundleFromURLCall = null;

String url = call.request().url().toString();

callback.onFailure(
DebugServerException.makeGeneric(
DebugServerException.makeGeneric(url,
"Could not connect to development server.",
"URL: " + call.request().url().toString(),
"URL: " + url,
e));
}

Expand Down Expand Up @@ -284,7 +286,7 @@ private void processBundleResult(
// Check for server errors. If the server error has the expected form, fail with more info.
if (statusCode != 200) {
String bodyString = body.readUtf8();
DebugServerException debugServerException = DebugServerException.parse(bodyString);
DebugServerException debugServerException = DebugServerException.parse(url, bodyString);
if (debugServerException != null) {
callback.onFailure(debugServerException);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package com.facebook.react.devsupport;

import android.content.Context;
import android.content.res.Resources;
import android.os.AsyncTask;
import android.os.Handler;
import android.os.Looper;
Expand Down Expand Up @@ -261,7 +262,7 @@ protected Boolean doInBackground(Void... ignore) {

public boolean doSync() {
try {
String attachToNuclideUrl = getInspectorAttachUrl(title);
String attachToNuclideUrl = getInspectorAttachUrl(context, title);
OkHttpClient client = new OkHttpClient();
Request request = new Request.Builder().url(attachToNuclideUrl).build();
client.newCall(request).execute();
Expand Down Expand Up @@ -367,11 +368,11 @@ private String getInspectorDeviceUrl() {
mPackageName);
}

private String getInspectorAttachUrl(String title) {
private String getInspectorAttachUrl(Context context, String title) {
return String.format(
Locale.US,
"http://%s/nuclide/attach-debugger-nuclide?title=%s&app=%s&device=%s",
AndroidInfoHelpers.getServerHost(),
AndroidInfoHelpers.getServerHost(context),
title,
mPackageName,
AndroidInfoHelpers.getFriendlyDeviceName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Locale;

import android.content.Context;
import android.content.res.Resources;
import android.os.Build;

import com.facebook.common.logging.FLog;
import com.facebook.react.R;

public class AndroidInfoHelpers {

Expand All @@ -23,9 +25,6 @@ public class AndroidInfoHelpers {

public static final String METRO_HOST_PROP_NAME = "metro.host";

private static final int DEBUG_SERVER_HOST_PORT = 8081;
private static final int INSPECTOR_PROXY_PORT = 8081;

private static final String TAG = AndroidInfoHelpers.class.getSimpleName();

private static boolean isRunningOnGenymotion() {
Expand All @@ -36,12 +35,24 @@ private static boolean isRunningOnStockEmulator() {
return Build.FINGERPRINT.contains("generic");
}

public static String getServerHost() {
return getServerIpAddress(DEBUG_SERVER_HOST_PORT);
public static String getServerHost(Integer port) {
return getServerIpAddress(port);
}

public static String getServerHost(Context context) {
return getServerIpAddress(getDevServerPort(context));
}

public static String getInspectorProxyHost() {
return getServerIpAddress(INSPECTOR_PROXY_PORT);
public static String getAdbReverseTcpCommand(Integer port) {
return "adb reverse tcp:" + port + " tcp:" + port;
}

public static String getAdbReverseTcpCommand(Context context) {
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.
Expand All @@ -54,6 +65,16 @@ public static String getFriendlyDeviceName() {
}
}

private static Integer getDevServerPort(Context context) {
Resources resources = context.getResources();
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@

import android.annotation.SuppressLint;
import android.app.UiModeManager;
import android.content.Context;
import android.content.res.Configuration;
import android.content.res.Resources;
import android.os.Build;
import android.provider.Settings.Secure;

import com.facebook.react.R;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContextBaseJavaModule;
import com.facebook.react.common.build.ReactBuildConfig;
Expand All @@ -35,9 +38,7 @@ public class AndroidInfoModule extends ReactContextBaseJavaModule {
public static final String NAME = "PlatformConstants";
private static final String IS_TESTING = "IS_TESTING";

public AndroidInfoModule(ReactApplicationContext reactContext) {
super(reactContext);
}
public AndroidInfoModule(ReactApplicationContext reactContext) { super(reactContext); }

/**
* See: https://developer.android.com/reference/android/app/UiModeManager.html#getCurrentModeType()
Expand Down Expand Up @@ -74,7 +75,7 @@ public String getName() {
constants.put("Fingerprint", Build.FINGERPRINT);
constants.put("Model", Build.MODEL);
if (ReactBuildConfig.DEBUG) {
constants.put("ServerHost", AndroidInfoHelpers.getServerHost());
constants.put("ServerHost", getServerHost());
}
constants.put("isTesting", "true".equals(System.getProperty(IS_TESTING))
|| isRunningScreenshotTest());
Expand All @@ -96,4 +97,12 @@ private Boolean isRunningScreenshotTest() {
return false;
}
}

private String getServerHost() {
Resources resources = getReactApplicationContext().getApplicationContext().getResources();

Integer devServerPort = resources.getInteger(R.integer.react_native_dev_server_port);

return AndroidInfoHelpers.getServerHost(devServerPort);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ rn_android_library(
react_native_target("java/com/facebook/react/bridge:bridge"),
react_native_target("java/com/facebook/react/common:common"),
react_native_target("java/com/facebook/react/module/annotations:annotations"),
react_native_target("res:systeminfo"),
],
exported_deps = [
":systeminfo-moduleless",
Expand All @@ -29,8 +30,10 @@ rn_android_library(
"PUBLIC",
],
deps = [
react_native_target("java/com/facebook/react/common:common"),
react_native_dep("libraries/fbcore/src/main/java/com/facebook/common/logging:logging"),
react_native_dep("third-party/java/infer-annotations:infer-annotations"),
react_native_dep("third-party/java/jsr-305:jsr-305"),
react_native_target("res:systeminfo"),
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@

package com.facebook.react.packagerconnection;

import javax.annotation.Nullable;

import android.content.Context;
import android.content.SharedPreferences;
import android.preference.PreferenceManager;
import android.text.TextUtils;

import javax.annotation.Nullable;

import com.facebook.common.logging.FLog;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.modules.systeminfo.AndroidInfoHelpers;
Expand All @@ -24,10 +24,12 @@ public class PackagerConnectionSettings {

private final SharedPreferences mPreferences;
private final String mPackageName;
private final Context mAppContext;

public PackagerConnectionSettings(Context applicationContext) {
mPreferences = PreferenceManager.getDefaultSharedPreferences(applicationContext);
mPackageName = applicationContext.getPackageName();
mAppContext = applicationContext;
}

public String getDebugServerHost() {
Expand All @@ -39,20 +41,20 @@ public String getDebugServerHost() {
return Assertions.assertNotNull(hostFromSettings);
}

String host = AndroidInfoHelpers.getServerHost();
String host = AndroidInfoHelpers.getServerHost(mAppContext);

if (host.equals(AndroidInfoHelpers.DEVICE_LOCALHOST)) {
FLog.w(
TAG,
"You seem to be running on device. Run 'adb reverse tcp:8081 tcp:8081' " +
"You seem to be running on device. Run '" + AndroidInfoHelpers.getAdbReverseTcpCommand(mAppContext) + "' " +
"to forward the debug server's port to the device.");
}

return host;
}

public String getInspectorServerHost() {
return AndroidInfoHelpers.getInspectorProxyHost();
return AndroidInfoHelpers.getInspectorProxyHost(mAppContext);
}

public @Nullable String getPackageName() {
Expand Down
9 changes: 9 additions & 0 deletions ReactAndroid/src/main/res/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,13 @@ rn_android_resource(
],
)

rn_android_resource(
name = "systeminfo",
package = "com.facebook.react",
res = "systeminfo",
visibility = [
"PUBLIC",
],
)

# New resource directories must be added to react-native-github/ReactAndroid/build.gradle
5 changes: 5 additions & 0 deletions ReactAndroid/src/main/res/systeminfo/values/values.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<integer name="react_native_dev_server_port">8081</integer>
<integer name="react_native_inspector_proxy_port">@integer/react_native_dev_server_port</integer>
</resources>
16 changes: 16 additions & 0 deletions react.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,22 @@ def reactRoot = file(config.root ?: "../../")
def inputExcludes = config.inputExcludes ?: ["android/**", "ios/**"]
def bundleConfig = config.bundleConfig ? "${reactRoot}/${config.bundleConfig}" : null ;

def reactNativeDevServerPort() {
def value = project.getProperties().get("reactNativeDevServerPort")
return value != null ? value : "8081"
}

def reactNativeInspectorProxyPort() {
def value = project.getProperties().get("reactNativeInspectorProxyPort")
return value != null ? value : reactNativeDevServerPort()
}

android {
buildTypes.all {
resValue "integer", "react_native_dev_server_port", reactNativeDevServerPort()
resValue "integer", "react_native_inspector_proxy_port", reactNativeInspectorProxyPort()
}
}

afterEvaluate {
def isAndroidLibrary = plugins.hasPlugin("com.android.library")
Expand Down

0 comments on commit df3ec68

Please sign in to comment.