-
Notifications
You must be signed in to change notification settings - Fork 577
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
React Native bridgeless #6718
Conversation
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
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
# '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/"' | ||
# ] |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
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"); |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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.
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 |
There was a problem hiding this comment.
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}" |
There was a problem hiding this comment.
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.
// externalNativeBuild { | ||
// cmake { | ||
// arguments "-DDEBUG=true" | ||
// } | ||
// } |
There was a problem hiding this comment.
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.
sourceSets { | ||
main { | ||
if (NEW_ARCH_ENABLED) { | ||
java.srcDirs += ["src/newarch"] | ||
} else { | ||
java.srcDirs += ["src/oldarch"] | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪄
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; | ||
} |
There was a problem hiding this comment.
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.
# '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/"' | ||
# ] |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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};`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
sourceSets { | ||
main { | ||
if (NEW_ARCH_ENABLED) { | ||
java.srcDirs += ["src/newarch"] | ||
} else { | ||
java.srcDirs += ["src/oldarch"] | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
getBinding(): Object; | |
readonly getBinding(): Object; |
getBinding(): Object; | ||
} | ||
|
||
export default TurboModuleRegistry.get<Spec>("Realm"); |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
60275c7
to
48e8eaa
Compare
Closing this in favour of #6737. |
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
DEBUG
definition gets propagated and the expected code is called when "invalidating".Compatibility
label is updated or copied from previous entryCOMPATIBILITY.md
package.json
s (if updating internal packages)Breaking
label has been applied or is not necessary