Skip to content

Commit

Permalink
Refactor: Remove AsyncDevSupportManager.loadJSBundleFromServer
Browse files Browse the repository at this point in the history
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
  • Loading branch information
RSNara authored and facebook-github-bot committed Apr 8, 2021
1 parent f525ec0 commit 5f0bf8b
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {}

Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 5f0bf8b

Please sign in to comment.