Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React Native bridgeless #6718

Closed
wants to merge 112 commits into from
Closed

React Native bridgeless #6718

wants to merge 112 commits into from

Conversation

kraenhansen
Copy link
Member

@kraenhansen kraenhansen commented Jun 12, 2024

What, How & Why?

This closes #6653 by introducing two implementations of the way we register our native module on both iOS and Android conditionally based on the enablement of the new architecture in the consuming app.

This needs to be rebased on main once #6650 gets merged.

☑️ ToDos

  • Ensure DEBUG definition gets propagated and the expected code is called when "invalidating".
  • Ensure the SSL helper gets passed correctly on Android.
  • Add testing of new / old architecture on CI.
  • 📝 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

commit 17e1661
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Tue Feb 6 13:18:02 2024 +0000

    Lint the shell script

commit 78b3ae7
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Mon Feb 5 11:49:56 2024 +0000

    Add all scripts to the package

commit e334dca
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Sun Feb 4 15:20:20 2024 +0100

    Fix potential release issues

commit fa5a982
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Sun Feb 4 15:16:21 2024 +0100

    Refactor integration tests jsi dep

commit efab94a
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Sun Feb 4 15:13:58 2024 +0100

    Revert wireit action change

commit 49d87c6
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Sun Feb 4 15:11:22 2024 +0100

    Add CHANGELOG entry

commit 1a92fb4
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Sun Feb 4 13:58:41 2024 +0100

    PR suggestions, etc

    * Rename generate script
    * Various Cleanup
    * Fix up the dependencies for the integration tests

commit 6cd3b15
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Sun Feb 4 13:47:49 2024 +0100

    Apply suggestions from code review

    Co-authored-by: Kræn Hansen <kraen.hansen@mongodb.com>
    Co-authored-by: Kenneth Geisshirt <kenneth.geisshirt@mongodb.com>
    Co-authored-by: LJ <81748770+elle-j@users.noreply.github.com>

commit 89c5c0f
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Sun Feb 4 13:40:18 2024 +0100

    Refactor Android builds for React Native (#6400)

    * Build Realm Android from Source

    * Derive React Native version from local RN package

    * Implement prefab to handle react native dependencies.  Refactor where possible

    * Refactor CI for Android

    * Fix pack issues

    * Possible ios cache issue

    * Fixed issue where build starts before ts is generated

    * disable wireit cache

    * refactor wireit cache and optimize workflow

    * Cleanup packagin

commit 8e01fbf
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Wed Jan 31 15:37:16 2024 +0100

    Revert upgrade to upload-artifact

commit 7477a81
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Wed Jan 31 14:41:10 2024 +0100

    Add cache for pod-install:ci

commit 8379c28
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Mon Jan 29 14:52:07 2024 +0100

    Attempt to fix CI
    Update node16 actions
    Refactor bundle step into generate artifact step

commit 58d62ff
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Mon Jan 29 14:47:53 2024 +0100

    PR feedback

commit 91e3a5f
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Mon Jan 29 14:45:22 2024 +0100

    Apply suggestions from code review

    Co-authored-by: LJ <81748770+elle-j@users.noreply.github.com>
    Co-authored-by: Kræn Hansen <kraen.hansen@mongodb.com>

commit cb4df32
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Mon Jan 29 14:44:31 2024 +0100

    Incorporate PR feedback

commit 43f7670
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Thu Jan 25 13:34:38 2024 +0100

    Fixes after review

commit c7ccfcf
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Thu Jan 25 11:05:43 2024 +0100

    Cleanup

commit 9e3d7c0
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Thu Jan 25 10:52:56 2024 +0100

    Updates from Review

commit 06ef7a9
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Wed Jan 24 12:45:50 2024 +0100

    Only generate jsi for react native builds

commit 33f29b0
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Wed Jan 24 12:33:57 2024 +0100

    PR comments and etc

    * Fixed some build issues
    * Reactivate ccache for integration tests builds

commit b67a299
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Fri Jan 19 14:59:23 2024 +0100

    Final fixes to iOS building from source

commit 0c66006
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Fri Jan 19 10:41:52 2024 +0100

    More documentation updates

commit 5ac6267
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Fri Jan 19 09:37:19 2024 +0100

    Prepare for public consumption

    * update the packed contents of the package
    * update relevant documentation

commit de9300f
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Wed Jan 17 16:39:07 2024 +0100

    Use an BUILD_REALM_CORE env var to build core from source

commit 14d9de6
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Wed Jan 17 16:24:14 2024 +0100

    Refactor ios builds

    * build from core prebuilds
    * phase script only called when input and outputs have changes
    * reinstalling pods will wipe build files, forcing a rebuild
    * generate input file list so that core can be rebuilt on changes
    * generate dummy libraries so that libraries can be generated
    * create build option which forces core to build from source
    * build from source if prebuild url is not reachable

commit 02ba32d
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Thu Jan 4 17:56:33 2024 +0100

    Remove xcframework and just use static libraries

commit d370cba
Author: Andrew Meyer <andrew.meyer@mongodb.com>
Date:   Sat Dec 30 11:09:10 2023 +0100

    Make builds for iOS work without downloading prebuilds
kraenhansen and others added 11 commits June 12, 2024 11:41
Co-authored-by: LJ <81748770+elle-j@users.noreply.github.com>
commit d0ddcbb
Author: Kræn Hansen <kraen.hansen@mongodb.com>
Date:   Fri May 31 22:03:27 2024 +0200

    Fixed @realm/react tests

commit fffa005
Author: Kræn Hansen <kraen.hansen@mongodb.com>
Date:   Fri May 31 21:58:22 2024 +0200

    Installing the React Native eslint plugin in the root

commit c233fd9
Author: Kræn Hansen <kraen.hansen@mongodb.com>
Date:   Fri May 31 16:23:34 2024 +0200

    Ran pod install

commit fefc108
Author: Kræn Hansen <kraen.hansen@mongodb.com>
Date:   Fri May 31 11:42:38 2024 +0200

    Disabling bridgeless on CI for now

commit 23c4150
Author: Kræn Hansen <kraen.hansen@mongodb.com>
Date:   Fri May 31 11:40:44 2024 +0200

    Exposing configuring bridgeless via environment variables

commit 8d3b410
Author: Kræn Hansen <kraen.hansen@mongodb.com>
Date:   Fri May 31 11:39:33 2024 +0200

    Upgrading React Native across packages, RNTA dependencies, hoisting RN TS config package
@kraenhansen kraenhansen self-assigned this Jun 12, 2024
@cla-bot cla-bot bot added the cla: yes label Jun 12, 2024
Comment on lines +75 to +84
# 'HEADER_SEARCH_PATHS' => [
# # Bootstrapper for React Native
# '"${PODS_TARGET_SRCROOT}/binding/apple/"',
# # Logger and JS-SDK specific helpers
# '"${PODS_TARGET_SRCROOT}/binding/"',
# # Platform specific helpers used by the generated binding code
# '"${PODS_TARGET_SRCROOT}/bindgen/src/"',
# # Platform independent helpers
# '"${PODS_TARGET_SRCROOT}/bindgen/vendor/realm-core/bindgen/src/"'
# ]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are now included as source files, to make them available via Xcode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you remove the commented out code later btw?

@@ -5,6 +5,7 @@ headers:

classes:
JsPlatformHelpers:
cppName: js::JsPlatformHelpers
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved all of the binding code into the realm::js namespace.

Comment on lines +37 to +46
if (Platform.OS === "android") {
// Getting the native module on Android will inject the Realm global
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const RealmNativeModule = NativeModules.Realm;
}
const { __injectedRealmBinding } = global;
if (__injectedRealmBinding) {
return __injectedRealmBinding;
}
throw new Error("Could not find the Realm native module. Please consult our troubleshooting guide: https://www.mongodb.com/docs/realm-sdks/js/latest/#md:troubleshooting-missing-binary");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the old architecture, we're injecting the native binding in __injectedRealmBinding (renamed in this PR from __RealmFuncs).

if(!nativeModule) {
throw new Error("Could not find the Realm binary. Please consult our troubleshooting guide: https://www.mongodb.com/docs/realm-sdks/js/latest/#md:troubleshooting-missing-binary");

const nativeBinding = getBinding();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed this from nativeModule, since the binding is returned from a function call on the "native module" and is not the module itself.

Comment on lines +1 to +150
jstring file_dir,
jobject assets) {

__android_log_print(ANDROID_LOG_VERBOSE, "Realm", "setDefaultRealmFileDirectory");

// Get the assetManager in case we want to copy files from the APK (assets)
AAssetManager* assetManager = AAssetManager_fromJava(env, assets);
if (assetManager == NULL) {
__android_log_print(ANDROID_LOG_ERROR, "Realm", "Error loading the AssetManager");
}
realm::js::set_asset_manager(assetManager);

// Setting the internal storage path for the application
const char* strFileDir = env->GetStringUTFChars(file_dir, NULL);
realm::js::JsPlatformHelpers::set_default_realm_file_directory(strFileDir);
env->ReleaseStringUTFChars(file_dir, strFileDir);

__android_log_print(ANDROID_LOG_DEBUG, "Realm", "Absolute path: %s",
realm::js::JsPlatformHelpers::default_realm_file_directory().c_str());
}

#if RCT_NEW_ARCH_ENABLED

extern "C"
JNIEXPORT void JNICALL
Java_io_realm_react_RealmReactPackage_registerModule(JNIEnv *env, jclass clazz) {
#if RCT_NEW_ARCH_ENABLED
__android_log_print(ANDROID_LOG_VERBOSE, "Realm", "Registering native module");
react::registerCxxModuleToGlobalModuleMap("Realm", [](std::shared_ptr<react::CallInvoker> js_invoker) {
__android_log_print(ANDROID_LOG_VERBOSE, "Realm", "Constructing native module");
return std::make_shared<realm::js::NativeRealmModule>(js_invoker);
});
#endif
}

#else

extern "C"
JNIEXPORT void JNICALL
Java_io_realm_react_RealmReactPackage_injectModuleIntoJSGlobal(JNIEnv *env, jclass clazz,
jlong runtime_pointer) {

__android_log_print(ANDROID_LOG_VERBOSE, "Realm", "Injecting module into JS global");
auto runtime = reinterpret_cast<jsi::Runtime*>(runtime_pointer);
if (!runtime) {
return;
}

auto exports = jsi::Object(*runtime);
realm_jsi_init(*runtime, exports);
runtime->global().setProperty(*runtime, "__injectedRealmBinding", exports);
}

extern "C"
JNIEXPORT void JNICALL
Java_io_realm_react_RealmReactPackage_injectCallInvoker(JNIEnv *env, jclass clazz,
jobject call_invoker) {
__android_log_print(ANDROID_LOG_VERBOSE, "Realm", "Getting JS call invoker");
// React Native uses the fbjni library for handling JNI, which has the concept of "hybrid objects",
// which are Java objects containing a pointer to a C++ object. The CallInvokerHolder, which has the
// invokeAsync method we want access to, is one such hybrid object.
// Rather than reworking our code to use fbjni throughout, this code unpacks the C++ object from the Java
// object `callInvokerHolderJavaObj` manually, based on reverse engineering the fbjni code.

// 1. Get the Java object referred to by the mHybridData field of the Java holder object
auto callInvokerHolderClass = env->GetObjectClass(call_invoker);
auto hybridDataField = env->GetFieldID(callInvokerHolderClass, "mHybridData", "Lcom/facebook/jni/HybridData;");
auto hybridDataObj = env->GetObjectField(call_invoker, hybridDataField);

// 2. Get the destructor Java object referred to by the mDestructor field from the myHybridData Java object
auto hybridDataClass = env->FindClass("com/facebook/jni/HybridData");
auto destructorField =
env->GetFieldID(hybridDataClass, "mDestructor", "Lcom/facebook/jni/HybridData$Destructor;");
auto destructorObj = env->GetObjectField(hybridDataObj, destructorField);

// 3. Get the mNativePointer field from the mDestructor Java object
auto destructorClass = env->FindClass("com/facebook/jni/HybridData$Destructor");
auto nativePointerField = env->GetFieldID(destructorClass, "mNativePointer", "J");
auto nativePointerValue = env->GetLongField(destructorObj, nativePointerField);

// 4. Cast the mNativePointer back to its C++ type
auto nativePointer = reinterpret_cast<facebook::react::CallInvokerHolder*>(nativePointerValue);

// 5. Store the JS call invoker for the workaround to use
realm::js::flush_ui_workaround::inject_js_call_invoker(nativePointer->getCallInvoker());
}

extern "C"
JNIEXPORT void JNICALL
Java_io_realm_react_RealmReactPackage_invalidateCaches(JNIEnv *env, jclass clazz) {
// Disable the flush ui workaround
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();
#endif
realm_jsi_invalidate_caches();
}

#endif
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved all the JNI code from the RealmReactModule to static native members of the RealmReactPackage (since their implementations weren't tied to the lifetime of the RealmReactModule anyway).

@@ -59,7 +61,7 @@ android {
"-DCMAKE_CXX_VISIBILITY_PRESET=hidden",
"-DREACT_NATIVE_ROOT_DIR=${REACT_NATIVE_ROOT_DIR}"
targets 'realm-js-android-binding'
cppFlags '-std=c++20'
cppFlags '-std=c++20', "-DRCT_NEW_ARCH_ENABLED=${NEW_ARCH_ENABLED}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used by the JNI code to switch implementation.

Comment on lines +76 to +80
// externalNativeBuild {
// cmake {
// arguments "-DDEBUG=true"
// }
// }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I need to remove this comment and ensure this gets propagated correctly.

Comment on lines +98 to +106
sourceSets {
main {
if (NEW_ARCH_ENABLED) {
java.srcDirs += ["src/newarch"]
} else {
java.srcDirs += ["src/oldarch"]
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the magic that controls which implementation is used based on the new arch being enabled for Android.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🪄

Comment on lines 48 to 63
JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void*)
{
JNIEnv* env;
if (vm->GetEnv((void**)&env, JNI_VERSION_1_6) != JNI_OK) {
return JNI_ERR;
}
else {
JniUtils::initialize(vm, JNI_VERSION_1_6);
}

// We do lookup the class in this Thread, since FindClass sometimes fails
// when issued from the sync client thread
ssl_helper_class = reinterpret_cast<jclass>(env->NewGlobalRef(env->FindClass("io/realm/react/util/SSLHelper")));

return JNI_VERSION_1_6;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I'll need to reimplement this in the new JNI code and test it.

Comment on lines +75 to +84
# 'HEADER_SEARCH_PATHS' => [
# # Bootstrapper for React Native
# '"${PODS_TARGET_SRCROOT}/binding/apple/"',
# # Logger and JS-SDK specific helpers
# '"${PODS_TARGET_SRCROOT}/binding/"',
# # Platform specific helpers used by the generated binding code
# '"${PODS_TARGET_SRCROOT}/bindgen/src/"',
# # Platform independent helpers
# '"${PODS_TARGET_SRCROOT}/bindgen/vendor/realm-core/bindgen/src/"'
# ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you remove the commented out code later btw?

namespace realm::js::JSI {
namespace {

struct FlushMicrotaskQueueGuard {
~FlushMicrotaskQueueGuard()
{
flush_ui_queue();
realm::js::flush_ui_workaround::flush_ui_queue();
Copy link
Contributor

@elle-j elle-j Jun 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like flush_ui_workaround doesn't exist in that namespace, or it can't find it.

out(`const ${native} = nativeModule.${method.id};`);
out(`const ${native} = nativeBinding.${method.id};`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +98 to +106
sourceSets {
main {
if (NEW_ARCH_ENABLED) {
java.srcDirs += ["src/newarch"]
} else {
java.srcDirs += ["src/oldarch"]
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🪄

auto &rt = *static_cast<facebook::jsi::Runtime *>(bridge.runtime);
auto exports = jsi::Object(rt);
realm_jsi_init(rt, exports, ^{
// Calling jsCallInvokver->invokeAsync tells React Native to execute the lambda passed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did these comments get moved elsewhere btw? Found it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


export interface Spec extends TurboModule {
// eslint-disable-next-line @typescript-eslint/ban-types
getBinding(): Object;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could probably make this readonly.

Suggested change
getBinding(): Object;
readonly getBinding(): Object;

getBinding(): Object;
}

export default TurboModuleRegistry.get<Spec>("Realm");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen both TurboModuleRegistry.get() and TurboModuleRegistry.getEnforcing() used but unsure if one is older or if there's even a difference.

@interface NativeRealmLoader()
@end

@implementation NativeRealmLoader
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if RCT_EXPORT_MODULE() is also necessary here based on this example.

@kraenhansen kraenhansen force-pushed the kh/build-optimizations-squashed branch from 60275c7 to 48e8eaa Compare June 14, 2024 19:45
Base automatically changed from kh/build-optimizations-squashed to main June 17, 2024 08:00
@kraenhansen
Copy link
Member Author

Closing this in favour of #6737.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support "bridgeless" mode by exposing a TurboModule on iOS
2 participants