From 5af7b7038c730289cd8420060c039197f45e7397 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Mon, 9 Oct 2023 14:28:03 -0700 Subject: [PATCH] Refactor BaseJavaModule and ReactContextBaseJavaModule (#39823) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39823 In this diff I'm refactoring BaseJavaModule and ReactContextBaseJavaModule to simplify class hierarchy. ReactContextBaseJavaModule will be deprecated in the new architecture bypass-github-export-checks Reviewed By: cortinico Differential Revision: D49930340 fbshipit-source-id: 602b5f3d2d926956c52b96b28815dae687fdad87 --- .../facebook/react/bridge/BaseJavaModule.java | 53 ++++++++++++++++++ .../bridge/ReactContextBaseJavaModule.java | 55 +++---------------- 2 files changed, 60 insertions(+), 48 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java index 3b4606afdd9f0f..6cab5695c26cb0 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java @@ -7,7 +7,14 @@ package com.facebook.react.bridge; +import static com.facebook.infer.annotation.ThreadConfined.ANY; + import androidx.annotation.Nullable; +import com.facebook.common.logging.FLog; +import com.facebook.infer.annotation.Assertions; +import com.facebook.infer.annotation.ThreadConfined; +import com.facebook.react.common.ReactConstants; +import com.facebook.react.common.build.ReactBuildConfig; import java.util.Map; /** @@ -44,6 +51,16 @@ public abstract class BaseJavaModule implements NativeModule { public static final String METHOD_TYPE_PROMISE = "promise"; public static final String METHOD_TYPE_SYNC = "sync"; + private final @Nullable ReactApplicationContext mReactApplicationContext; + + public BaseJavaModule() { + this(null); + } + + public BaseJavaModule(@Nullable ReactApplicationContext reactContext) { + mReactApplicationContext = reactContext; + } + /** @return a map of constants this module exports to JS. Supports JSON types. */ public @Nullable Map getConstants() { return null; @@ -70,4 +87,40 @@ public void onCatalystInstanceDestroy() {} public void invalidate() { onCatalystInstanceDestroy(); } + + /** Subclasses can use this method to access catalyst context passed as a constructor. */ + protected final ReactApplicationContext getReactApplicationContext() { + return Assertions.assertNotNull( + mReactApplicationContext, + "Tried to get ReactApplicationContext even though NativeModule wasn't instantiated with one"); + } + + /** + * Subclasses can use this method to access catalyst context passed as a constructor. Use this + * version to check that the underlying CatalystInstance is active before returning, and + * automatically have SoftExceptions or debug information logged for you. Consider using this + * whenever calling ReactApplicationContext methods that require the Catalyst instance be alive. + * + *

This can return null at any time, but especially during teardown methods it's + * possible/likely. + * + *

Threading implications have not been analyzed fully yet, so assume this method is not + * thread-safe. + */ + @ThreadConfined(ANY) + protected @Nullable final ReactApplicationContext getReactApplicationContextIfActiveOrWarn() { + if (mReactApplicationContext.hasActiveReactInstance()) { + return mReactApplicationContext; + } + + // We want to collect data about how often this happens, but SoftExceptions will cause a crash + // in debug mode, which isn't usually desirable. + String msg = "React Native Instance has already disappeared: requested by " + getName(); + if (ReactBuildConfig.DEBUG) { + FLog.w(ReactConstants.TAG, msg); + } else { + ReactSoftExceptionLogger.logSoftException(ReactConstants.TAG, new RuntimeException(msg)); + } + return null; + } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java index 6aacb484a53647..105303ebd01c23 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java @@ -7,65 +7,22 @@ package com.facebook.react.bridge; -import static com.facebook.infer.annotation.ThreadConfined.ANY; - import android.app.Activity; import androidx.annotation.Nullable; -import com.facebook.common.logging.FLog; -import com.facebook.infer.annotation.Assertions; -import com.facebook.infer.annotation.ThreadConfined; -import com.facebook.react.common.ReactConstants; -import com.facebook.react.common.build.ReactBuildConfig; +import com.facebook.react.common.annotations.DeprecatedInNewArchitecture; /** * Base class for Catalyst native modules that require access to the {@link ReactContext} instance. */ +@DeprecatedInNewArchitecture public abstract class ReactContextBaseJavaModule extends BaseJavaModule { - private final @Nullable ReactApplicationContext mReactApplicationContext; - public ReactContextBaseJavaModule() { - mReactApplicationContext = null; + super(null); } public ReactContextBaseJavaModule(@Nullable ReactApplicationContext reactContext) { - mReactApplicationContext = reactContext; - } - - /** Subclasses can use this method to access catalyst context passed as a constructor. */ - protected final ReactApplicationContext getReactApplicationContext() { - return Assertions.assertNotNull( - mReactApplicationContext, - "Tried to get ReactApplicationContext even though NativeModule wasn't instantiated with one"); - } - - /** - * Subclasses can use this method to access catalyst context passed as a constructor. Use this - * version to check that the underlying CatalystInstance is active before returning, and - * automatically have SoftExceptions or debug information logged for you. Consider using this - * whenever calling ReactApplicationContext methods that require the Catalyst instance be alive. - * - *

This can return null at any time, but especially during teardown methods it's - * possible/likely. - * - *

Threading implications have not been analyzed fully yet, so assume this method is not - * thread-safe. - */ - @ThreadConfined(ANY) - protected @Nullable final ReactApplicationContext getReactApplicationContextIfActiveOrWarn() { - if (mReactApplicationContext.hasActiveReactInstance()) { - return mReactApplicationContext; - } - - // We want to collect data about how often this happens, but SoftExceptions will cause a crash - // in debug mode, which isn't usually desirable. - String msg = "Catalyst Instance has already disappeared: requested by " + this.getName(); - if (ReactBuildConfig.DEBUG) { - FLog.w(ReactConstants.TAG, msg); - } else { - ReactSoftExceptionLogger.logSoftException(ReactConstants.TAG, new RuntimeException(msg)); - } - return null; + super(reactContext); } /** @@ -78,7 +35,9 @@ protected final ReactApplicationContext getReactApplicationContext() { * call this method whenever you actually need the Activity and make sure to check for {@code * null}. */ + @DeprecatedInNewArchitecture( + message = "Use 'getReactApplicationContext.getCurrentActivity() instead.") protected @Nullable final Activity getCurrentActivity() { - return mReactApplicationContext.getCurrentActivity(); + return getReactApplicationContext().getCurrentActivity(); } }