From 5f0bf8b2e9ef7464a18105ff86d79b4bbae53abe Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Wed, 7 Apr 2021 19:36:16 -0700 Subject: [PATCH] Refactor: Remove AsyncDevSupportManager.loadJSBundleFromServer Summary: ## Rationale - AsyncDevSupportManager.loadSplitBundleFromServer() is an override of DevSupportManager.loadSplitBundleFromServer(), which is used by the bridge. However, AsyncDevSupportManager.loadJSBundleFromServer() has no bridge analogue. This is confusing: Are the methods in AsyncDevSupportManager Venice overrides for bridge related methods? It's easy to think yes, but the answer is no. - AsyncDevSupportManager.loadJSBundleFromServer() is an additional layer of indirection that provides very little value: all it does it create the JSBundleLoader, and call onReactContextCreated. However, it does so in 11 lines of very confusing code. A discussion we don't have to have now: Inheritance hierarchies are very difficult to understand and de-tangle. So, instead of using inheritance to make DevSupportManager work with Venice (via AsyncDevSupportManager), should we just refactor DevSupportManager so that it can be customized to work with Venice? Changelog: [Internal] Reviewed By: JoshuaGross, mdvacca Differential Revision: D27577591 fbshipit-source-id: b64dcd65e9a7c85b89443d860d441a0635547916 --- .../react/devsupport/DevSupportManagerBase.java | 7 ++----- .../react/devsupport/DisabledDevSupportManager.java | 4 ++++ .../devsupport/interfaces/BundleLoadCallback.java | 12 ++++++++++++ .../devsupport/interfaces/DevSupportManager.java | 2 ++ 4 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/BundleLoadCallback.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java index 2932fecc5dad56..785b78f8050984 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java @@ -43,6 +43,7 @@ import com.facebook.react.common.ShakeDetector; import com.facebook.react.common.futures.SimpleSettableFuture; import com.facebook.react.devsupport.DevServerHelper.PackagerCommandListener; +import com.facebook.react.devsupport.interfaces.BundleLoadCallback; import com.facebook.react.devsupport.interfaces.DevBundleDownloadListener; import com.facebook.react.devsupport.interfaces.DevOptionHandler; import com.facebook.react.devsupport.interfaces.DevSplitBundleCallback; @@ -1151,11 +1152,7 @@ public void run() { }); } - protected interface BundleLoadCallback { - void onSuccess(); - } - - protected void reloadJSFromServer(final String bundleURL, final BundleLoadCallback callback) { + public void reloadJSFromServer(final String bundleURL, final BundleLoadCallback callback) { ReactMarker.logMarker(ReactMarkerConstants.DOWNLOAD_START); mDevLoadingViewController.showForUrl(bundleURL); diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DisabledDevSupportManager.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DisabledDevSupportManager.java index be29234308c7a8..b3738654b61540 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DisabledDevSupportManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DisabledDevSupportManager.java @@ -12,6 +12,7 @@ import com.facebook.react.bridge.DefaultNativeModuleCallExceptionHandler; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReadableArray; +import com.facebook.react.devsupport.interfaces.BundleLoadCallback; import com.facebook.react.devsupport.interfaces.DevOptionHandler; import com.facebook.react.devsupport.interfaces.DevSplitBundleCallback; import com.facebook.react.devsupport.interfaces.DevSupportManager; @@ -131,6 +132,9 @@ public void handleReloadJS() {} @Override public void reloadJSFromServer(String bundleURL) {} + @Override + public void reloadJSFromServer(final String bundleURL, final BundleLoadCallback callback) {} + @Override public void loadSplitBundleFromServer(String bundlePath, DevSplitBundleCallback callback) {} diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/BundleLoadCallback.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/BundleLoadCallback.java new file mode 100644 index 00000000000000..40495dfdbf9341 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/BundleLoadCallback.java @@ -0,0 +1,12 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.devsupport.interfaces; + +public interface BundleLoadCallback { + void onSuccess(); +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.java index 265f2da731eaf1..6fc8020a41030f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.java @@ -69,6 +69,8 @@ public interface DevSupportManager extends NativeModuleCallExceptionHandler { void reloadJSFromServer(final String bundleURL); + void reloadJSFromServer(final String bundleURL, final BundleLoadCallback callback); + void loadSplitBundleFromServer(String bundlePath, DevSplitBundleCallback callback); void isPackagerRunning(PackagerStatusCallback callback);