Skip to content

Commit

Permalink
Configure fragment-based nav to work with useEffect() (#44105)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #44105

Previously, we skipped calling the ReactDelegate callback functions when the ReavtNavigationFragment was destroyed because it set the current activity to null when we need to actually keep the reference to the activity. However, skipping this entirely also skips core clean up logic, such as running the return() function for useEffect().

Instead of skipping the callback function entirely, we just need to make sure we don't set mCurrentActivity to null. I followed D30504616 to configure an option to not set mCurrentActivity to null if mKeepActivity flag is set on the ReactInstanceManager.

Changelog: [Internal]

Reviewed By: keoskate

Differential Revision: D56167533

fbshipit-source-id: cb3620e21599683e0c6bbc5a6a9c4f384fdbcc51
  • Loading branch information
janeli-100005636499545 authored and facebook-github-bot committed Apr 17, 2024
1 parent 63e202f commit 2509eb7
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 6 deletions.
3 changes: 3 additions & 0 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ public class com/facebook/react/ReactInstanceManagerBuilder {
public fun setJSExceptionHandler (Lcom/facebook/react/bridge/JSExceptionHandler;)Lcom/facebook/react/ReactInstanceManagerBuilder;
public fun setJSMainModulePath (Ljava/lang/String;)Lcom/facebook/react/ReactInstanceManagerBuilder;
public fun setJavaScriptExecutorFactory (Lcom/facebook/react/bridge/JavaScriptExecutorFactory;)Lcom/facebook/react/ReactInstanceManagerBuilder;
public fun setKeepActivity (Z)Lcom/facebook/react/ReactInstanceManagerBuilder;
public fun setLazyViewManagersEnabled (Z)Lcom/facebook/react/ReactInstanceManagerBuilder;
public fun setMinNumShakes (I)Lcom/facebook/react/ReactInstanceManagerBuilder;
public fun setMinTimeLeftInFrameForNonBatchedOperationMs (I)Lcom/facebook/react/ReactInstanceManagerBuilder;
Expand All @@ -309,6 +310,7 @@ public abstract class com/facebook/react/ReactNativeHost {
public fun clear ()V
protected fun createReactInstanceManager ()Lcom/facebook/react/ReactInstanceManager;
protected final fun getApplication ()Landroid/app/Application;
protected fun getBaseReactInstanceManagerBuilder ()Lcom/facebook/react/ReactInstanceManagerBuilder;
protected fun getBundleAssetName ()Ljava/lang/String;
protected fun getChoreographerProvider ()Lcom/facebook/react/internal/ChoreographerProvider;
protected fun getDevLoadingViewManager ()Lcom/facebook/react/devsupport/interfaces/DevLoadingViewManager;
Expand Down Expand Up @@ -1125,6 +1127,7 @@ public abstract class com/facebook/react/bridge/ReactContext : android/content/C
public fun isOnUiQueueThread ()Z
public fun onActivityResult (Landroid/app/Activity;IILandroid/content/Intent;)V
public fun onHostDestroy ()V
public fun onHostDestroy (Z)V
public fun onHostPause ()V
public fun onHostResume (Landroid/app/Activity;)V
public fun onNewIntent (Landroid/app/Activity;Landroid/content/Intent;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ public interface ReactInstanceEventListener
private final DevSupportManager mDevSupportManager;
private final boolean mUseDeveloperSupport;
private final boolean mRequireActivity;
private final boolean mKeepActivity;
private final @Nullable NotThreadSafeBridgeIdleDebugListener mBridgeIdleDebugListener;
private final Object mReactContextLock = new Object();
private @Nullable volatile ReactContext mCurrentReactContext;
Expand Down Expand Up @@ -228,6 +229,7 @@ public static ReactInstanceManagerBuilder builder() {
boolean useDeveloperSupport,
DevSupportManagerFactory devSupportManagerFactory,
boolean requireActivity,
boolean keepActivity,
@Nullable NotThreadSafeBridgeIdleDebugListener bridgeIdleDebugListener,
LifecycleState initialLifecycleState,
JSExceptionHandler jSExceptionHandler,
Expand Down Expand Up @@ -257,6 +259,7 @@ public static ReactInstanceManagerBuilder builder() {
mPackages = new ArrayList<>();
mUseDeveloperSupport = useDeveloperSupport;
mRequireActivity = requireActivity;
mKeepActivity = keepActivity;
Systrace.beginSection(
Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstanceManager.initDevSupportManager");
mDevSupportManager =
Expand Down Expand Up @@ -700,7 +703,9 @@ public void onHostDestroy() {
}

moveToBeforeCreateLifecycleState();
mCurrentActivity = null;
if (!mKeepActivity) {
mCurrentActivity = null;
}
}

/**
Expand Down Expand Up @@ -810,7 +815,7 @@ private synchronized void moveToBeforeCreateLifecycleState() {
mLifecycleState = LifecycleState.BEFORE_RESUME;
}
if (mLifecycleState == LifecycleState.BEFORE_RESUME) {
currentContext.onHostDestroy();
currentContext.onHostDestroy(mKeepActivity);
}
}
mLifecycleState = LifecycleState.BEFORE_CREATE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public class ReactInstanceManagerBuilder {
private boolean mUseDeveloperSupport;
private @Nullable DevSupportManagerFactory mDevSupportManagerFactory;
private boolean mRequireActivity;
private boolean mKeepActivity;
private @Nullable LifecycleState mInitialLifecycleState;
private @Nullable JSExceptionHandler mJSExceptionHandler;
private @Nullable Activity mCurrentActivity;
Expand Down Expand Up @@ -208,6 +209,11 @@ public ReactInstanceManagerBuilder setRequireActivity(boolean requireActivity) {
return this;
}

public ReactInstanceManagerBuilder setKeepActivity(boolean keepActivity) {
mKeepActivity = keepActivity;
return this;
}

/**
* When the {@link SurfaceDelegateFactory} is provided, it will be used for native modules to get
* a {@link SurfaceDelegate} to interact with the platform specific surface that they that needs
Expand Down Expand Up @@ -345,6 +351,7 @@ public ReactInstanceManager build() {
? new DefaultDevSupportManagerFactory()
: mDevSupportManagerFactory,
mRequireActivity,
mKeepActivity,
mBridgeIdleDebugListener,
Assertions.assertNotNull(mInitialLifecycleState, "Initial lifecycle state was not set"),
mJSExceptionHandler,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ public void clear() {

protected ReactInstanceManager createReactInstanceManager() {
ReactMarker.logMarker(ReactMarkerConstants.BUILD_REACT_INSTANCE_MANAGER_START);
ReactInstanceManagerBuilder builder = getBaseReactInstanceManagerBuilder();
ReactMarker.logMarker(ReactMarkerConstants.BUILD_REACT_INSTANCE_MANAGER_END);
return builder.build();
}

protected ReactInstanceManagerBuilder getBaseReactInstanceManagerBuilder() {
ReactInstanceManagerBuilder builder =
ReactInstanceManager.builder()
.setApplication(mApplication)
Expand Down Expand Up @@ -102,9 +108,7 @@ protected ReactInstanceManager createReactInstanceManager() {
} else {
builder.setBundleAssetName(Assertions.assertNotNull(getBundleAssetName()));
}
ReactInstanceManager reactInstanceManager = builder.build();
ReactMarker.logMarker(ReactMarkerConstants.BUILD_REACT_INSTANCE_MANAGER_END);
return reactInstanceManager;
return builder;
}

/** Get the {@link RedBoxHandler} to send RedBox-related callbacks to. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public abstract class ReactContext extends ContextWrapper {

@DoNotStrip
public interface RCTDeviceEventEmitter extends JavaScriptModule {

void emit(@NonNull String eventName, @Nullable Object data);
}

Expand Down Expand Up @@ -385,7 +386,19 @@ public void onHostDestroy() {
handleException(e);
}
}
mCurrentActivity = null;
resetCurrentActivity(false);
}

@ThreadConfined(UI)
public void onHostDestroy(boolean keepActivity) {
onHostDestroy();
resetCurrentActivity(keepActivity);
}

private void resetCurrentActivity(boolean keepActivity) {
if (!keepActivity) {
mCurrentActivity = null;
}
}

/** Destroy this instance, making it unusable. */
Expand Down Expand Up @@ -513,6 +526,7 @@ public void handleException(Exception e) {
}

public class ExceptionHandlerWrapper implements JSExceptionHandler {

@Override
public void handleException(Exception e) {
ReactContext.this.handleException(e);
Expand Down

0 comments on commit 2509eb7

Please sign in to comment.