[kotlinify] Kotlinify BaseJavaModule #51207

l2hyunwoo · 2025-05-09T02:15:51Z

Summary:

Issues from #50513

Migrate BaseJavaModule.java from Java to Kotlin

Discussion

I think it's also possible to call getReactApplicationContext as a property getter of reactApplicationContext rather than as a function call, and I believe that writing it this way preserves the unique syntax of Kotlin. What do you think?
If it sounds, I'll make other PR to use property getter of non-nullable ReactApplicationContext

AS-IS

@Nullsafe(Nullsafe.Mode.LOCAL)
@StableReactNativeAPI
public abstract class BaseJavaModule(
  private val reactApplicationContext: ReactApplicationContext? = null
) : NativeModule {
  // ....
  protected fun getReactApplicationContext(): ReactApplicationContext =
    requireNotNull(reactApplicationContext) {
      "Tried to get ReactApplicationContext even though NativeModule wasn't instantiated with one"
    }
}

TO-BE

@Nullsafe(Nullsafe.Mode.LOCAL)
@StableReactNativeAPI
public abstract class BaseJavaModule(
  private val _reactApplicationContext: ReactApplicationContext? = null
) : NativeModule {
  protected val reactApplicationContext: ReactApplicationContext
    get() = requireNotNull(reactApplicationContext) {
      "Tried to get ReactApplicationContext even though NativeModule wasn't instantiated with one"
    }
}

Changelog:

[ANDROID] [CHANGED] Migrate BaseJavaModule from Java to Kotlin

Test Plan:

yarn android
yarn test-android

yarn android

yarn test-android

l2hyunwoo · 2025-05-09T14:53:57Z

w: file:///__w/react-native/react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt:242:35 This synthetic property is based on the getter function 'fun getReactApplicationContextIfActiveOrWarn(): ReactApplicationContext?' from Kotlin. In the future, synthetic properties will be available only if the base getter function came from Java. Consider replacing this property access with a 'getReactApplicationContextIfActiveOrWarn()' function call.
w: file:///__w/react-native/react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt:249:5 This synthetic property is based on the getter function 'fun getReactApplicationContext(): ReactApplicationContext' from Kotlin. In the future, synthetic properties will be available only if the base getter function came from Java. Consider replacing this property access with a 'getReactApplicationContext()' function call.
w: file:///__w/react-native/react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt:362:39 This synthetic property is based on the getter function 'fun getReactApplicationContextIfActiveOrWarn(): ReactApplicationContext?' from Kotlin. In the future, synthetic properties will be available only if the base getter function came from Java. Consider replacing this property access with a 'getReactApplicationContextIfActiveOrWarn()' function call.
w: file:///__w/react-native/react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt:445:35 This synthetic property is based on the getter function 'fun getReactApplicationContextIfActiveOrWarn(): ReactApplicationContext?' from Kotlin. In the future, synthetic properties will be available only if the base getter function came from Java. Consider replacing this property access with a 'getReactApplicationContextIfActiveOrWarn()' function call.

It should be fixed on other commit

mateoguzmana

Thanks for migrating this file!

Could you please remove any change that is not related to the migration of the file itself? It will make it easier for us to review and analyse whether the migration has potential breaking changes in the ecosystem.

mateoguzmana · 2025-05-10T08:18:48Z

...ages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/toast/ToastModule.kt

@@ -32,15 +32,15 @@ internal class ToastModule(reactContext: ReactApplicationContext) :
  override fun show(message: String?, durationDouble: Double) {
    val duration = durationDouble.toInt()
    UiThreadUtil.runOnUiThread {
-      Toast.makeText(getReactApplicationContext(), message, duration).show()
+      Toast.makeText(reactApplicationContext, message, duration).show()


Are these changes related to the migration? If not, let's please take them out – preferably you could send them over in a separate PR

I apply your code review! Thanks for review it.

mateoguzmana

Thanks for addressing the other comments! I have some concerns about the breaking changes this might introduce.

I have an example of a file that maintains backwards compatibility, perhaps we can try something like that in this case: InteropEvent.kt

mateoguzmana · 2025-05-11T17:53:57Z

...-native/ReactAndroid/src/main/java/com/facebook/react/modules/appearance/AppearanceModule.kt

@@ -17,6 +17,7 @@ import com.facebook.react.bridge.UiThreadUtil
 import com.facebook.react.module.annotations.ReactModule

 /** Module that exposes the user's preferred color scheme. */
+@Suppress("SYNTHETIC_PROPERTY_WITHOUT_JAVA_ORIGIN")


Why do we need these suppressions?

mateoguzmana · 2025-05-11T17:57:39Z

...ive/ReactCommon/react/nativemodule/samples/platform/android/NativeSampleTurboModuleSpec.java

@@ -41,19 +41,19 @@ public NativeSampleTurboModuleSpec(ReactApplicationContext reactContext) {
  }

  protected final void emitOnPress() {
-    mEventEmitterCallback.invoke("onPress");
+    getEventEmitterCallback().invoke("onPress");


Normally when we have to do these changes after a migration, it indicates it is a breaking change. I've checked and this specific property is used in other open source packages: See

Could you try to find a way to maintain backwards compatibility? we might have to expose a getter separately, but leaving the property with the same name so it can also be accessed like the change you did here

mateoguzmana · 2025-05-11T17:58:25Z

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.kt

@@ -109,7 +109,7 @@ internal class JavaModuleWrapper(
      val baseJavaModule = module

      Systrace.beginSection(TRACE_TAG_REACT, "module.getConstants")
-      val map = baseJavaModule.constants
+      val map = baseJavaModule.getConstants()


Similar here, we would need to find a way to maintain the access to the constants backwards compatible

mateoguzmana · 2025-05-11T17:59:13Z

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.kt

-  @DoNotStrip
-  protected void setEventEmitterCallback(CxxCallbackImpl eventEmitterCallback) {
-    mEventEmitterCallback = eventEmitterCallback;
+  public companion object {


Nit: let's try to leave the original reference from where the strings where taken from

Suggested change

public companion object {

public companion object {

// taken from Libraries/Utilities/MessageQueue.js

- revoke property access - Remove warning as marking with an annotation

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels May 9, 2025

l2hyunwoo force-pushed the feature/kotlinfy-basejavamodule branch 2 times, most recently from eeb882e to bac454f Compare May 9, 2025 03:24

l2hyunwoo marked this pull request as draft May 9, 2025 03:33

l2hyunwoo force-pushed the feature/kotlinfy-basejavamodule branch from bac454f to 50b5f1e Compare May 9, 2025 05:38

l2hyunwoo marked this pull request as ready for review May 9, 2025 05:39

l2hyunwoo force-pushed the feature/kotlinfy-basejavamodule branch 2 times, most recently from fea09af to 3193c7b Compare May 9, 2025 10:30

l2hyunwoo force-pushed the feature/kotlinfy-basejavamodule branch 3 times, most recently from 9f59aee to 1df590a Compare May 9, 2025 15:37

mateoguzmana requested changes May 10, 2025

View reviewed changes

mateoguzmana mentioned this pull request May 10, 2025

☂️ Help us Kotlin-ify React Native - Round 3 #50513

Open

19 tasks

l2hyunwoo force-pushed the feature/kotlinfy-basejavamodule branch 5 times, most recently from 4a40e10 to 86ac511 Compare May 11, 2025 04:29

mateoguzmana requested changes May 11, 2025

View reviewed changes

l2hyunwoo added 6 commits May 15, 2025 19:42

Kotlinify BaseJavaModule

5c9a9f6

Remove build error

bdb47ba

Remove property access warning

b525d81

Replace function call with property access

9162d1d

Suppress warnings on test classes

f9178c1

Apply code review

e390cdc

- revoke property access - Remove warning as marking with an annotation

l2hyunwoo force-pushed the feature/kotlinfy-basejavamodule branch from 86ac511 to e390cdc Compare May 15, 2025 10:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[kotlinify] Kotlinify BaseJavaModule #51207

[kotlinify] Kotlinify BaseJavaModule #51207

l2hyunwoo commented May 9, 2025 •

edited

Loading

l2hyunwoo commented May 9, 2025

mateoguzmana left a comment

mateoguzmana May 10, 2025

l2hyunwoo May 10, 2025

mateoguzmana left a comment

mateoguzmana May 11, 2025

mateoguzmana May 11, 2025 •

edited

Loading

mateoguzmana May 11, 2025

mateoguzmana May 11, 2025

	public companion object {
	public companion object {
	// taken from Libraries/Utilities/MessageQueue.js

[kotlinify] Kotlinify BaseJavaModule #51207

Are you sure you want to change the base?

[kotlinify] Kotlinify BaseJavaModule #51207

Conversation

l2hyunwoo commented May 9, 2025 • edited Loading

Summary:

Discussion

AS-IS

TO-BE

Changelog:

Test Plan:

yarn android

yarn test-android

l2hyunwoo commented May 9, 2025

mateoguzmana left a comment

Choose a reason for hiding this comment

mateoguzmana May 10, 2025

Choose a reason for hiding this comment

l2hyunwoo May 10, 2025

Choose a reason for hiding this comment

mateoguzmana left a comment

Choose a reason for hiding this comment

mateoguzmana May 11, 2025

Choose a reason for hiding this comment

mateoguzmana May 11, 2025 • edited Loading

Choose a reason for hiding this comment

mateoguzmana May 11, 2025

Choose a reason for hiding this comment

mateoguzmana May 11, 2025

Choose a reason for hiding this comment

l2hyunwoo commented May 9, 2025 •

edited

Loading

mateoguzmana May 11, 2025 •

edited

Loading