From 59ae0487ce05333ba5df6d2244f7eca7ecf82625 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Tue, 13 Sep 2022 06:42:37 -0700 Subject: [PATCH] Further simplify the New App Template by don't requiring the dynamic library name (#34671) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/34671 I'm simplifying the template further by: - Do not expose a configurable dynamic library name. Let's use `appmodules` and still allow the users to configure it, if needed. - Move all the initialization logic inside the `JNI_OnLoad` method - Cleanup the `DefaultReactNativeHost` to don't require a dynamic library name but just a boolean. Changelog: [Android] [Changed] - Further simplify the New App Template by don't requiring the dynamic library name Reviewed By: cipolleschi Differential Revision: D39462948 fbshipit-source-id: 737733fc263162a0baf3b7a451e48b8616679d3b --- .../defaults/DefaultComponentsRegistry.kt | 2 +- .../react/defaults/DefaultNativeEntryPoint.kt | 26 +++------- .../react/defaults/DefaultReactNativeHost.kt | 26 +++++----- .../DefaultTurboModuleManagerDelegate.kt | 2 +- packages/rn-tester/android/app/build.gradle | 9 ---- .../react/uiapp/RNTesterApplication.java | 6 +-- .../android/app/src/main/jni/CMakeLists.txt | 4 +- .../android/app/src/main/jni/OnLoad.cpp | 50 +++++-------------- template/android/app/build.gradle | 9 ---- .../java/com/helloworld/MainApplication.java | 13 ++--- .../android/app/src/main/jni/CMakeLists.txt | 2 +- template/android/app/src/main/jni/OnLoad.cpp | 48 ++++-------------- 12 files changed, 53 insertions(+), 144 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultComponentsRegistry.kt b/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultComponentsRegistry.kt index cd0f46e359c5df..4eb6401b35b21e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultComponentsRegistry.kt +++ b/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultComponentsRegistry.kt @@ -16,7 +16,7 @@ import com.facebook.react.fabric.ComponentFactory * implementation of its native methods. * * This class works together with the [DefaultNativeEntryPoint] and it's C++ implementation is - * hosted inside the React Native framwork + * hosted inside the React Native framework */ @DoNotStrip class DefaultComponentsRegistry diff --git a/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultNativeEntryPoint.kt b/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultNativeEntryPoint.kt index e5acf77e88dd07..02db04658aed62 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultNativeEntryPoint.kt +++ b/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultNativeEntryPoint.kt @@ -7,8 +7,6 @@ package com.facebook.react.defaults -import com.facebook.jni.HybridData -import com.facebook.proguard.annotations.DoNotStrip import com.facebook.soloader.SoLoader /** @@ -18,22 +16,14 @@ import com.facebook.soloader.SoLoader * This class needs to be invoked as `DefaultNativeEntryPoint.load("...")` by passing the name of * the dynamic library to load. * - * This class works together with the [DefaultNativeEntryPoint] and it's C++ implementation is - * hosted inside the React Native framework + * By default it loads a library called `appmodules`. `appmodules` is a convention used to refer to + * the application dynamic library. If changed here should be updated also inside the template. */ -@DoNotStrip -class DefaultNativeEntryPoint @DoNotStrip private constructor() { - - @DoNotStrip private val hybridData: HybridData = initHybrid() - - @DoNotStrip private external fun initHybrid(): HybridData - - companion object { - @JvmStatic - fun load(dynamicLibraryName: String) { - SoLoader.loadLibrary("react_newarchdefaults") - SoLoader.loadLibrary(dynamicLibraryName) - DefaultNativeEntryPoint() - } +object DefaultNativeEntryPoint { + @JvmStatic + @JvmOverloads + fun load(dynamicLibraryName: String = "appmodules") { + SoLoader.loadLibrary("react_newarchdefaults") + SoLoader.loadLibrary(dynamicLibraryName) } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactNativeHost.kt b/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactNativeHost.kt index 06a69bc6095e9e..9bbaee4ebbfadb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactNativeHost.kt +++ b/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactNativeHost.kt @@ -25,29 +25,27 @@ abstract class DefaultReactNativeHost protected constructor(application: Applica override fun getReactPackageTurboModuleManagerDelegateBuilder(): ReactPackageTurboModuleManagerDelegate.Builder? = - dynamicLibraryName?.let { - // If the user provided a dynamic library name, we assume they want to load - // the default ReactPackageTurboModuleManagerDelegate + if (isNewArchEnabled) { DefaultTurboModuleManagerDelegate.Builder() + } else { + null } override fun getJSIModulePackage(): JSIModulePackage? = - dynamicLibraryName?.let { - // If the user provided a dynamic library name, we assume they want to load - // the default JSIModulePackage + if (isNewArchEnabled) { DefaultJSIModulePackage(this) + } else { + null } /** - * Returns the name of the dynamic library used by app on the New Architecture. This is generally - * "_appmodules" or just "appmodules" + * Returns whether the user wants to use the New Architecture or not. * - * If null, we will assume you're not using the New Architecture and will not attempt to load any - * dynamic library at runtime. + * If true, we will load the default JSI Module Package and TurboModuleManagerDelegate needed to + * enable the New Architecture * - * If set, we'll take care of create a TurboModuleManagerDelegate that will load the library you - * specified. + * If false, the app will not attempt to load the New Architecture modules. */ - protected open val dynamicLibraryName: String? - get() = null + protected open val isNewArchEnabled: Boolean + get() = false } diff --git a/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultTurboModuleManagerDelegate.kt b/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultTurboModuleManagerDelegate.kt index b3cd8223920f24..714bb654c77c30 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultTurboModuleManagerDelegate.kt +++ b/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultTurboModuleManagerDelegate.kt @@ -18,7 +18,7 @@ import com.facebook.react.bridge.ReactApplicationContext * [ReactPackageTurboModuleManagerDelegate] for new apps in Open Source. * * This class works together with the [DefaultNativeEntryPoint] and it's C++ implementation is - * hosted inside the React Native framwork + * hosted inside the React Native framework */ class DefaultTurboModuleManagerDelegate private constructor(context: ReactApplicationContext, packages: List) : diff --git a/packages/rn-tester/android/app/build.gradle b/packages/rn-tester/android/app/build.gradle index 198b913466cdb5..0c058a8804b94f 100644 --- a/packages/rn-tester/android/app/build.gradle +++ b/packages/rn-tester/android/app/build.gradle @@ -123,13 +123,6 @@ def reactNativeArchitectures() { return value ? value.split(",") : ["armeabi-v7a", "x86", "x86_64", "arm64-v8a"] } -/** - * The name of the dynamic library for this application. This will contain all the - * compiled C++ code and will be loaded at runtime. - * For RN tester is "rntester_appmodules" so that we'll have a `librntester_appmodules.so` to load. - */ -def dynamicLibraryName = "rntester_appmodules" - android { buildToolsVersion = "31.0.0" compileSdkVersion 31 @@ -161,7 +154,6 @@ android { versionName "1.0" testBuildType System.getProperty('testBuildType', 'debug') // This will later be used to control the test apk build type testInstrumentationRunner 'androidx.test.runner.AndroidJUnitRunner' - buildConfigField("String", "DYNAMIC_LIBRARY_NAME", "\"$dynamicLibraryName\"") } signingConfigs { release { @@ -249,7 +241,6 @@ android { "-DPROJECT_BUILD_DIR=$buildDir", "-DREACT_ANDROID_DIR=$reactAndroidProjectDir", "-DREACT_ANDROID_BUILD_DIR=$reactAndroidBuildDir", - "-DTARGET_NAME=$dynamicLibraryName", "-DANDROID_STL=c++_shared" } } diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java index 589cb4df56c82d..cdc5ac628282a9 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java @@ -112,8 +112,8 @@ public List createViewManagers( } @Override - public String getDynamicLibraryName() { - return BuildConfig.DYNAMIC_LIBRARY_NAME; + protected boolean isNewArchEnabled() { + return true; } }; @@ -123,7 +123,7 @@ public void onCreate() { ReactFontManager.getInstance().addCustomFont(this, "Rubik", R.font.rubik); super.onCreate(); SoLoader.init(this, /* native exopackage */ false); - DefaultNativeEntryPoint.load(BuildConfig.DYNAMIC_LIBRARY_NAME); + DefaultNativeEntryPoint.load(); ReactNativeFlipper.initializeFlipper(this, getReactNativeHost().getReactInstanceManager()); } diff --git a/packages/rn-tester/android/app/src/main/jni/CMakeLists.txt b/packages/rn-tester/android/app/src/main/jni/CMakeLists.txt index 496b839d767198..e263a03350c428 100644 --- a/packages/rn-tester/android/app/src/main/jni/CMakeLists.txt +++ b/packages/rn-tester/android/app/src/main/jni/CMakeLists.txt @@ -5,8 +5,8 @@ cmake_minimum_required(VERSION 3.13) -# Define the library name here. -project(${TARGET_NAME}) +# Define the application dynamic library name here. +project(appmodules) include(${REACT_ANDROID_DIR}/cmake-utils/ReactNative-application.cmake) diff --git a/packages/rn-tester/android/app/src/main/jni/OnLoad.cpp b/packages/rn-tester/android/app/src/main/jni/OnLoad.cpp index 73b838cffbaf0b..df67d208f0b7aa 100644 --- a/packages/rn-tester/android/app/src/main/jni/OnLoad.cpp +++ b/packages/rn-tester/android/app/src/main/jni/OnLoad.cpp @@ -14,18 +14,16 @@ #include #include -namespace facebook { -namespace react { - void registerComponents( - std::shared_ptr registry) { - registry->add(concreteComponentDescriptorProvider< - RNTMyNativeViewComponentDescriptor>()); + std::shared_ptr + registry) { + registry->add(facebook::react::concreteComponentDescriptorProvider< + facebook::react::RNTMyNativeViewComponentDescriptor>()); } -std::shared_ptr provideModules( +std::shared_ptr provideModules( const std::string &name, - const JavaTurboModule::InitParams ¶ms) { + const facebook::react::JavaTurboModule::InitParams ¶ms) { auto module = AppSpecs_ModuleProvider(name, params); if (module != nullptr) { return module; @@ -37,35 +35,11 @@ std::shared_ptr provideModules( return rncore_ModuleProvider(name, params); } -class RNTesterNativeEntryPoint - : public facebook::jni::HybridClass { - public: - constexpr static auto kJavaDescriptor = - "Lcom/facebook/react/defaults/DefaultNativeEntryPoint;"; - - static jni::local_ref initHybrid(jni::alias_ref) { - DefaultTurboModuleManagerDelegate::moduleProvidersFromEntryPoint = - &provideModules; - DefaultComponentsRegistry::registerComponentDescriptorsFromEntryPoint = - ®isterComponents; - return makeCxxInstance(); - } - - static void registerNatives() { - registerHybrid({ - makeNativeMethod("initHybrid", RNTesterNativeEntryPoint::initHybrid), - }); - } - - private: - friend HybridBase; - using HybridBase::HybridBase; -}; - -} // namespace react -} // namespace facebook - JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *) { - return facebook::jni::initialize( - vm, [] { facebook::react::RNTesterNativeEntryPoint::registerNatives(); }); + return facebook::jni::initialize(vm, [] { + facebook::react::DefaultTurboModuleManagerDelegate:: + moduleProvidersFromEntryPoint = &provideModules; + facebook::react::DefaultComponentsRegistry:: + registerComponentDescriptorsFromEntryPoint = ®isterComponents; + }); } diff --git a/template/android/app/build.gradle b/template/android/app/build.gradle index d67b6f86cc9a5c..f0ad51910b4325 100644 --- a/template/android/app/build.gradle +++ b/template/android/app/build.gradle @@ -129,13 +129,6 @@ def reactNativeArchitectures() { return value ? value.split(",") : ["armeabi-v7a", "x86", "x86_64", "arm64-v8a"] } -/** - * The name of the dynamic library for this application. This will contain all the - * compiled C++ code and will be loaded at runtime. - * The default is "appmodules" so that we'll have a `libappmodules.so` to load. - */ -def dynamicLibraryName = "appmodules" - android { ndkVersion rootProject.ext.ndkVersion @@ -148,7 +141,6 @@ android { versionCode 1 versionName "1.0" buildConfigField "boolean", "IS_NEW_ARCHITECTURE_ENABLED", isNewArchitectureEnabled().toString() - buildConfigField "String", "DYNAMIC_LIBRARY_NAME", "\"$dynamicLibraryName\"" if (isNewArchitectureEnabled()) { // We configure the CMake build only if you decide to opt-in for the New Architecture. @@ -157,7 +149,6 @@ android { arguments "-DPROJECT_BUILD_DIR=$buildDir", "-DREACT_ANDROID_DIR=$rootDir/../node_modules/react-native/ReactAndroid", "-DREACT_ANDROID_BUILD_DIR=$rootDir/../node_modules/react-native/ReactAndroid/build", - "-DTARGET_NAME=$dynamicLibraryName", "-DANDROID_STL=c++_shared" } } diff --git a/template/android/app/src/main/java/com/helloworld/MainApplication.java b/template/android/app/src/main/java/com/helloworld/MainApplication.java index 7b9729aa53600f..ccf1addb2b9d9b 100644 --- a/template/android/app/src/main/java/com/helloworld/MainApplication.java +++ b/template/android/app/src/main/java/com/helloworld/MainApplication.java @@ -35,15 +35,8 @@ protected String getJSMainModuleName() { } @Override - public String getDynamicLibraryName() { - // If you enabled the New Architecture, you need to return the name of the - // dynamic library to load (usually 'appmodule'). This is configured - // in your build.gradle file. - if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) { - return BuildConfig.DYNAMIC_LIBRARY_NAME; - } else { - return null; - } + protected boolean isNewArchEnabled() { + return BuildConfig.IS_NEW_ARCHITECTURE_ENABLED; } }; @@ -60,7 +53,7 @@ public void onCreate() { // If you opted-in for the New Architecture, we enable the TurboModule system // and load the native entry point for this app. ReactFeatureFlags.useTurboModules = true; - DefaultNativeEntryPoint.load(BuildConfig.DYNAMIC_LIBRARY_NAME); + DefaultNativeEntryPoint.load(); } ReactNativeFlipper.initializeFlipper(this, getReactNativeHost().getReactInstanceManager()); } diff --git a/template/android/app/src/main/jni/CMakeLists.txt b/template/android/app/src/main/jni/CMakeLists.txt index ab770ef3412da9..25c9c208afbcd4 100644 --- a/template/android/app/src/main/jni/CMakeLists.txt +++ b/template/android/app/src/main/jni/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.13) # Define the library name here. -project(${TARGET_NAME}) +project(appmodules) # This file includes all the necessary to let you build your application with the New Architecture. include(${REACT_ANDROID_DIR}/cmake-utils/ReactNative-application.cmake) diff --git a/template/android/app/src/main/jni/OnLoad.cpp b/template/android/app/src/main/jni/OnLoad.cpp index 6a0f654bcac31f..5a04a57ae01114 100644 --- a/template/android/app/src/main/jni/OnLoad.cpp +++ b/template/android/app/src/main/jni/OnLoad.cpp @@ -4,11 +4,9 @@ #include #include -namespace facebook { -namespace react { - void registerComponents( - std::shared_ptr registry) { + std::shared_ptr + registry) { // Custom Fabric Components go here. You can register custom // components coming from your App or from 3rd party libraries here. // @@ -16,12 +14,12 @@ void registerComponents( // AocViewerComponentDescriptor>()); // By default we just use the components autolinked by RN CLI - rncli_registerProviders(registry); + facebook::react::rncli_registerProviders(registry); } -std::shared_ptr provideModules( +std::shared_ptr provideModules( const std::string &name, - const JavaTurboModule::InitParams ¶ms) { + const facebook::react::JavaTurboModule::InitParams ¶ms) { // Here you can provide your own module provider for TurboModules coming from // either your application or from external libraries. The approach to follow // is similar to the following (for a library called `samplelibrary`): @@ -33,40 +31,14 @@ std::shared_ptr provideModules( // return rncore_ModuleProvider(moduleName, params); // By default we just use the module providers autolinked by RN CLI - return rncli_ModuleProvider(name, params); + return facebook::react::rncli_ModuleProvider(name, params); } -class MainApplicationNativeEntryPoint - : public facebook::jni::HybridClass { - public: - constexpr static auto kJavaDescriptor = - "Lcom/facebook/react/defaults/DefaultNativeEntryPoint;"; - - static jni::local_ref initHybrid(jni::alias_ref) { - DefaultTurboModuleManagerDelegate::moduleProvidersFromEntryPoint = - &provideModules; - DefaultComponentsRegistry::registerComponentDescriptorsFromEntryPoint = - ®isterComponents; - return makeCxxInstance(); - } - - static void registerNatives() { - registerHybrid({ - makeNativeMethod( - "initHybrid", MainApplicationNativeEntryPoint::initHybrid), - }); - } - - private: - friend HybridBase; - using HybridBase::HybridBase; -}; - -} // namespace react -} // namespace facebook - JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *) { return facebook::jni::initialize(vm, [] { - facebook::react::MainApplicationNativeEntryPoint::registerNatives(); + facebook::react::DefaultTurboModuleManagerDelegate:: + moduleProvidersFromEntryPoint = &provideModules; + facebook::react::DefaultComponentsRegistry:: + registerComponentDescriptorsFromEntryPoint = ®isterComponents; }); }