Skip to content

Commit

Permalink
DevServerHelper should not depend on internal ctor parameter (#37265)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #37265

DevServerHelper was having a constructor parameter as `DevInternalSettings` which is effectively internal. This should not be the case as that class is Internal as was bleeding out of the public API.

I've updated the primary constructor to take instead:

```
  public DevServerHelper(
          DeveloperSettings settings,
          String packageName,
          InspectorPackagerConnection.BundleStatusProvider bundleStatusProvider,
          PackagerConnectionSettings packagerConnectionSettings) {
```

This is breaking change for users that were depending on the Internal class.

Changelog:
[Android] [Removed] - DevServerHelper should not depend on internal ctor parameter

Reviewed By: mdvacca

Differential Revision: D45600283

fbshipit-source-id: b8386ba8650546c3a8f5cec8574ef98f9e2f0494
  • Loading branch information
cortinico authored and facebook-github-bot committed May 5, 2023
1 parent 8aa2281 commit 8c2a451
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
import com.facebook.react.common.ReactConstants;
import com.facebook.react.devsupport.interfaces.DevBundleDownloadListener;
import com.facebook.react.devsupport.interfaces.PackagerStatusCallback;
import com.facebook.react.modules.debug.interfaces.DeveloperSettings;
import com.facebook.react.modules.systeminfo.AndroidInfoHelpers;
import com.facebook.react.packagerconnection.FileIoHandler;
import com.facebook.react.packagerconnection.JSPackagerClient;
import com.facebook.react.packagerconnection.NotificationOnlyHandler;
import com.facebook.react.packagerconnection.PackagerConnectionSettings;
import com.facebook.react.packagerconnection.ReconnectingWebSocket.ConnectionCallback;
import com.facebook.react.packagerconnection.RequestHandler;
import com.facebook.react.packagerconnection.RequestOnlyHandler;
Expand Down Expand Up @@ -99,7 +101,10 @@ public String typeID() {
}
}

private final DevInternalSettings mSettings;
private final DeveloperSettings mSettings;

private final PackagerConnectionSettings mPackagerConnectionSettings;

private final OkHttpClient mClient;
private final BundleDownloader mBundleDownloader;
private final PackagerStatusCheck mPackagerStatusCheck;
Expand All @@ -110,10 +115,12 @@ public String typeID() {
private InspectorPackagerConnection.BundleStatusProvider mBundlerStatusProvider;

public DevServerHelper(
DevInternalSettings settings,
DeveloperSettings developerSettings,
String packageName,
InspectorPackagerConnection.BundleStatusProvider bundleStatusProvider) {
mSettings = settings;
InspectorPackagerConnection.BundleStatusProvider bundleStatusProvider,
PackagerConnectionSettings packagerConnectionSettings) {
mSettings = developerSettings;
mPackagerConnectionSettings = packagerConnectionSettings;
mBundlerStatusProvider = bundleStatusProvider;
mClient =
new OkHttpClient.Builder()
Expand Down Expand Up @@ -181,10 +188,7 @@ public void onDisconnected() {

mPackagerClient =
new JSPackagerClient(
clientId,
mSettings.getPackagerConnectionSettings(),
handlers,
onPackagerConnectedCallback);
clientId, mPackagerConnectionSettings, handlers, onPackagerConnectedCallback);
mPackagerClient.init();

return null;
Expand Down Expand Up @@ -277,14 +281,14 @@ public String getWebsocketProxyURL() {
return String.format(
Locale.US,
"ws://%s/debugger-proxy?role=client",
mSettings.getPackagerConnectionSettings().getDebugServerHost());
mPackagerConnectionSettings.getDebugServerHost());
}

private String getInspectorDeviceUrl() {
return String.format(
Locale.US,
"http://%s/inspector/device?name=%s&app=%s",
mSettings.getPackagerConnectionSettings().getInspectorServerHost(),
mPackagerConnectionSettings.getInspectorServerHost(),
AndroidInfoHelpers.getFriendlyDeviceName(),
mPackageName);
}
Expand Down Expand Up @@ -315,8 +319,7 @@ public void downloadBundleFromURL(
/** @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(mSettings.getPackagerConnectionSettings().getDebugServerHost());
String host = Assertions.assertNotNull(mPackagerConnectionSettings.getDebugServerHost());
int portOffset = host.lastIndexOf(':');
if (portOffset > -1) {
return "localhost" + host.substring(portOffset);
Expand Down Expand Up @@ -362,8 +365,7 @@ private String createBundleURL(
}

private String createBundleURL(String mainModuleID, BundleType type) {
return createBundleURL(
mainModuleID, type, mSettings.getPackagerConnectionSettings().getDebugServerHost());
return createBundleURL(mainModuleID, type, mPackagerConnectionSettings.getDebugServerHost());
}

private static String createResourceURL(String host, String resourcePath) {
Expand All @@ -372,18 +374,15 @@ private static String createResourceURL(String host, String resourcePath) {

public String getDevServerBundleURL(final String jsModulePath) {
return createBundleURL(
jsModulePath,
BundleType.BUNDLE,
mSettings.getPackagerConnectionSettings().getDebugServerHost());
jsModulePath, BundleType.BUNDLE, mPackagerConnectionSettings.getDebugServerHost());
}

public String getDevServerSplitBundleURL(String jsModulePath) {
return createSplitBundleURL(
jsModulePath, mSettings.getPackagerConnectionSettings().getDebugServerHost());
return createSplitBundleURL(jsModulePath, mPackagerConnectionSettings.getDebugServerHost());
}

public void isPackagerRunning(final PackagerStatusCallback callback) {
String host = mSettings.getPackagerConnectionSettings().getDebugServerHost();
String host = mPackagerConnectionSettings.getDebugServerHost();
if (host == null) {
FLog.w(ReactConstants.TAG, "No packager host configured.");
callback.onPackagerStatusFetched(false);
Expand All @@ -396,7 +395,7 @@ private String createLaunchJSDevtoolsCommandUrl() {
return String.format(
Locale.US,
"http://%s/launch-js-devtools",
mSettings.getPackagerConnectionSettings().getDebugServerHost());
mPackagerConnectionSettings.getDebugServerHost());
}

public void launchJSDevtools() {
Expand Down Expand Up @@ -443,8 +442,7 @@ public String getJSBundleURLForRemoteDebugging(String mainModuleName) {
public @Nullable File downloadBundleResourceFromUrlSync(
final String resourcePath, final File outputFile) {
final String resourceURL =
createResourceURL(
mSettings.getPackagerConnectionSettings().getDebugServerHost(), resourcePath);
createResourceURL(mPackagerConnectionSettings.getDebugServerHost(), resourcePath);
final Request request = new Request.Builder().url(resourceURL).build();

try (Response response = mClient.newCall(request).execute()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,10 @@ public DevSupportManagerBase(
mBundleStatus = new InspectorPackagerConnection.BundleStatus();
mDevServerHelper =
new DevServerHelper(
mDevSettings, mApplicationContext.getPackageName(), () -> mBundleStatus);
mDevSettings,
mApplicationContext.getPackageName(),
() -> mBundleStatus,
mDevSettings.getPackagerConnectionSettings());
mBundleDownloadListener = devBundleDownloadListener;

// Prepare shake gesture detector (will be started/stopped from #reload)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,8 @@ public void onInternalSettingsChanged() {}
new DevServerHelper(
mDevSettings,
applicationContext.getPackageName(),
new InspectorPackagerConnection.BundleStatusProvider() {
@Override
public InspectorPackagerConnection.BundleStatus getBundleStatus() {
return mBundleStatus;
}
});
(InspectorPackagerConnection.BundleStatusProvider) () -> mBundleStatus,
mDevSettings.getPackagerConnectionSettings());
}

@Override
Expand Down

0 comments on commit 8c2a451

Please sign in to comment.