RJS-2815: React Native bridgeless support (take 2) #6737

kraenhansen · 2024-06-18T20:18:37Z

What, How & Why?

This closes #6653 by introducing a new implementations of the way we register our native module on both iOS and Android. I suggest relying on the legacy NativeModules compatibility layer for now, since the current state of pure C++ TurboModules are largely undocumented and experimental: We could revisit this decision in the future by introducing an alternative implementation, conditional on the new architecture being enabled in the app.

This approach aligns with others vendors having exposing their native C++ / Rust based native libraries through JSI.

This still injects the binding into the JS global, from where the "JS wrapper" of our native module is able to read and delete it again. With these changes, we're now deferring the injection of this (and the loading of our .so on Android) until the very last moment it's needed, essentially getting the benefits of deferred loading that TurboModules bring:

realm-js/packages/realm/binding/android/src/main/java/io/realm/react/RealmReactModule.java

Lines 63 to 66 in d774f35

    
           // Calling loadLibrary multiple times will be ignored 
        
           // We do it here instead of statically to allow the app load faster if Realm isn't accessed. 
        
           // Effectively emulating the behaviour of TurboModules. 
        
           SoLoader.loadLibrary("realm");

Since we're implementing and injecting our binding onto the a global using JSI, we're not relying on the legacy bridge compatibility layer for every call from JS into C++ and I therefore expect this to be as fast as a TurboModule would be.

I did attempt to inject the "JS call invoker" for the "UI flush workaround" only if bridgeless is disabled, as facebook/react-native#43396 fixed this for bridgeless and we don't want to pay the cost if it isn't needed, but it didn't work. We could still experiment with using the JSI Runtime::drainMicrotasks() API if available.

☑️ ToDos

📝 Changelog entry
📝 Compatibility label is updated or copied from previous entry
📝 Update COMPATIBILITY.md
🚦 Tests
📦 Updated internal package version in consuming package.jsons (if updating internal packages)
📱 Check the React Native/other sample apps work if necessary
💥 Breaking label has been applied or is not necessary

kraenhansen · 2024-07-15T10:10:56Z

integration-tests/environments/react-native-test-app/harness/runner.js

@@ -139,7 +139,7 @@ async function run(spawnLogcat) {
    if (PLATFORM === "android") {
      // Start the log cat (skipping any initial pid from an old run)
      if (spawnLogcat) {
-        logcat.start("com.realmreactnativetests", true).catch(console.error);
+        logcat.start("com.microsoft.reacttestapp", true).catch(console.error);


Technically no longer needed, but it does fix an issue of spawning logcat when running locally.

kraenhansen · 2024-07-15T10:14:58Z

packages/realm/binding/android/src/main/cpp/io_realm_react_RealmReactModule.h

@@ -1,45 +0,0 @@
-/* DO NOT EDIT THIS FILE - it is machine generated */


This file is not needed as no code is referring to these symbols externally.

kraenhansen · 2024-07-15T10:16:15Z

packages/realm/binding/jsi/flush_ui_queue_workaround.cpp

+
+#include <ReactCommon/CallInvoker.h>
+
+namespace realm::js::flush_ui_workaround {


As part of #6718 I did a refactor of the "flush UI workaround" which I decided to keep in this PR, since I felt it was a nice increase in the separation of concerns.

kneth

My comments are not blocking but merely suggestions and questions to clarify a few things (for me)

kneth · 2024-07-16T07:43:42Z

packages/realm/RealmJS.podspec

  s.source_files           = 'binding/jsi/*.cpp',
-                             'binding/apple/*.mm'
+                             'binding/apple/platform.mm',


Is the any reason for not using all .mm files?

Nope. I don't remember why I changed this. I've updated this to be 'binding/apple/*.mm' again.

kneth · 2024-07-16T07:46:16Z

packages/realm/binding/android/src/main/cpp/io_realm_react_RealmReactModule.cpp

 {
-    __android_log_print(ANDROID_LOG_VERBOSE, "JSRealm", "setDefaultRealmFileDirectory");
+
+    __android_log_print(ANDROID_LOG_VERBOSE, "Realm", "install");


Can we use Core's logger instead?

Core isn't necessarily loaded into memory at the point of calling code in this file, so I'd rather - not to keep things simple 🙂

kneth · 2024-07-16T09:00:35Z

packages/realm/binding/android/src/main/java/io/realm/react/RealmReactModule.java

+        // Calling loadLibrary multiple times will be ignored
+        // We do it here instead of statically to allow the app load faster if Realm isn't accessed.
+        // Effectively emulating the behaviour of TurboModules.
+        SoLoader.loadLibrary("realm");


Should we wrap it in a try/catch to provide a better error mesage?

I imagine the SoLoader will provide sufficient information - we can add to this if we see common patterns emerge, but right now I don't know what I would write to make it more actionable for developers 🤔 It used to be the case that people forgot to react-native link but that's a thing of the past.

kneth · 2024-07-16T09:02:25Z

packages/realm/binding/android/src/main/cpp/io_realm_react_RealmReactModule.cpp

+    realm::js::flush_ui_workaround::reset_js_call_invoker();
+    __android_log_print(ANDROID_LOG_VERBOSE, "Realm", "Invalidating caches");
+#if DEBUG
+    realm_jsi_close_sync_sessions();


Please include the comments as in RealmReactModule.m

kraenhansen · 2024-07-16T11:51:08Z

I included a fix with acb518d solving #6787 in the process.

ferrannp · 2024-08-15T15:19:01Z

I have still not being able to migrate my project to new architecture but super happy to see this ongoing. Just writing to say thank you @kraenhansen !

kraenhansen · 2024-08-16T12:01:08Z

@ferrannp Honestly, a comment like that means more to us than you might know 🙂 Thank you!

kraenhansen added the T-Internal label Jun 18, 2024

kraenhansen self-assigned this Jun 18, 2024

cla-bot bot added the cla: yes label Jun 18, 2024

kraenhansen force-pushed the kh/rn-bridgeless-2 branch 3 times, most recently from e448000 to 1b09689 Compare June 24, 2024 09:58

This was referenced Jun 25, 2024

Iterating bridgeless support for Android #6755

Closed

Test on Android using "release" configuration #6769

Merged

kraenhansen added 10 commits July 15, 2024 12:06

Refactoring Android native module

12cc9a6

Upgrading RNTA and RN

267e3eb

Refactoring iOS native module

9eb86e2

Enabling bridgeless on CI

cdec6c9

Deleting the old setBridge dependent code

9d77230

Injecting js_call invoker only if microtasks are disabled

07e238e

Fixed linting issues

c6c711a

Enabling new arch and bridgeles from gradle properties

e221068

Re-introducing the workaround

0fe1c32

Fixed logcat spawning

d774f35

kraenhansen force-pushed the kh/rn-bridgeless-2 branch from 1b09689 to a7d26ea Compare July 15, 2024 10:07

kraenhansen commented Jul 15, 2024

View reviewed changes

kraenhansen force-pushed the kh/rn-bridgeless-2 branch from a7d26ea to d774f35 Compare July 15, 2024 10:11

kraenhansen commented Jul 15, 2024

View reviewed changes

kraenhansen marked this pull request as ready for review July 15, 2024 15:02

kneth approved these changes Jul 16, 2024

View reviewed changes

kraenhansen changed the title ~~React Native bridgeless support (take 2)~~ RJS-2815: React Native bridgeless support (take 2) Jul 16, 2024

kraenhansen added 3 commits July 16, 2024 12:37

Updating locks

6c39be3

Adding a comment

bf83773

Fixed lint warnings

acb518d

kraenhansen force-pushed the kh/rn-bridgeless-2 branch from 50f526b to acb518d Compare July 16, 2024 11:44

Less explicit source_files in Podspec

bc3edf2

kraenhansen merged commit 96f7c04 into main Jul 16, 2024
34 checks passed

kraenhansen deleted the kh/rn-bridgeless-2 branch July 16, 2024 20:46

This was referenced Jul 17, 2024

Gradle lint error with 12.10.0 and yarn errors with 12.11.1 #6787

Closed

React Native bridgeless #6718

Closed

kraenhansen mentioned this pull request Jul 24, 2024

Realm does not build on iOS with use_frameworks! :linkage => :static as of version 12.12.0 #6808

Closed

github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RJS-2815: React Native bridgeless support (take 2) #6737

RJS-2815: React Native bridgeless support (take 2) #6737

kraenhansen commented Jun 18, 2024 •

edited

Loading

kraenhansen Jul 15, 2024

kraenhansen Jul 15, 2024

kraenhansen Jul 15, 2024 •

edited

Loading

kneth left a comment

kneth Jul 16, 2024

kraenhansen Jul 16, 2024 •

edited

Loading

kneth Jul 16, 2024

kraenhansen Jul 16, 2024 •

edited

Loading

kneth Jul 16, 2024

kraenhansen Jul 16, 2024

kneth Jul 16, 2024

kraenhansen commented Jul 16, 2024 •

edited

Loading

ferrannp commented Aug 15, 2024

kraenhansen commented Aug 16, 2024 •

edited

Loading

	// Calling loadLibrary multiple times will be ignored
	// We do it here instead of statically to allow the app load faster if Realm isn't accessed.
	// Effectively emulating the behaviour of TurboModules.
	SoLoader.loadLibrary("realm");

		@@ -1,45 +0,0 @@
		/* DO NOT EDIT THIS FILE - it is machine generated */


		#include <ReactCommon/CallInvoker.h>

		namespace realm::js::flush_ui_workaround {

RJS-2815: React Native bridgeless support (take 2) #6737

RJS-2815: React Native bridgeless support (take 2) #6737

Conversation

kraenhansen commented Jun 18, 2024 • edited Loading

What, How & Why?

☑️ ToDos

kraenhansen Jul 15, 2024

Choose a reason for hiding this comment

kraenhansen Jul 15, 2024

Choose a reason for hiding this comment

kraenhansen Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

kneth left a comment

Choose a reason for hiding this comment

kneth Jul 16, 2024

Choose a reason for hiding this comment

kraenhansen Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

kneth Jul 16, 2024

Choose a reason for hiding this comment

kraenhansen Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

kneth Jul 16, 2024

Choose a reason for hiding this comment

kraenhansen Jul 16, 2024

Choose a reason for hiding this comment

kneth Jul 16, 2024

Choose a reason for hiding this comment

kraenhansen commented Jul 16, 2024 • edited Loading

ferrannp commented Aug 15, 2024

kraenhansen commented Aug 16, 2024 • edited Loading

kraenhansen commented Jun 18, 2024 •

edited

Loading

kraenhansen Jul 15, 2024 •

edited

Loading

kraenhansen Jul 16, 2024 •

edited

Loading

kraenhansen Jul 16, 2024 •

edited

Loading

kraenhansen commented Jul 16, 2024 •

edited

Loading

kraenhansen commented Aug 16, 2024 •

edited

Loading