From da358d0ec7a492edb804b9cdce70e7516ee518ae Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Fri, 5 May 2023 09:58:07 -0700 Subject: [PATCH] DevServerHelper should not depend on internal ctor parameter (#37265) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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: e73139dbdf5f2505201b2d2c8b5a9143b7e207ba --- .../react/devsupport/DevServerHelper.java | 44 +++++++++---------- .../devsupport/DevSupportManagerBase.java | 5 ++- .../devsupport/PerftestDevSupportManager.java | 8 +--- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java index a3fadbba2961a0..58f7f8de0acd0a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java @@ -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; @@ -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; @@ -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() @@ -181,10 +188,7 @@ public void onDisconnected() { mPackagerClient = new JSPackagerClient( - clientId, - mSettings.getPackagerConnectionSettings(), - handlers, - onPackagerConnectedCallback); + clientId, mPackagerConnectionSettings, handlers, onPackagerConnectedCallback); mPackagerClient.init(); return null; @@ -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); } @@ -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); @@ -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) { @@ -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); @@ -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() { @@ -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()) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java index cd6471491d089a..b393db1675623d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java @@ -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) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PerftestDevSupportManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PerftestDevSupportManager.java index 2be621fc38ff6a..d15eba912d99d8 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PerftestDevSupportManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PerftestDevSupportManager.java @@ -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