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

chore(Android): add autolinking on Fabric #1585

Merged
merged 36 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
d5a9109
chore: update codegen config configuration in main package.json
kkafar Sep 10, 2022
4f84579
chore: remove files with custom autolinking C++ code
kkafar Sep 10, 2022
da0faed
chore: remove custom linking code from gradle build
kkafar Sep 10, 2022
e177bf4
chore: apply recomended directory structure to common cpp code
kkafar Sep 12, 2022
d3b7f73
chore: copy codegened implementation of cpp code for RNSScreen
kkafar Sep 12, 2022
da63061
chore: remove ComponentsDescriptors.h
kkafar Sep 12, 2022
7963fd0
chore: remove ShadowNodes{.cpp,.h} as we provide custom implementation
kkafar Sep 12, 2022
882f032
chore: add JSI_EXPORT macros to shadow node decl
kkafar Sep 12, 2022
57f3346
chore: add module provider cpp code
kkafar Sep 12, 2022
d54857f
chore: add custom RN CLI config
kkafar Sep 12, 2022
0ccc2ea
fix: add all components descriptors & specify cmakeListsPath
kkafar Sep 12, 2022
b8ccfa3
chore: build custom & generated code with cmake
kkafar Sep 12, 2022
1a00f6a
chore: add some debug statements to custom Cmake
kkafar Sep 13, 2022
e1d5f3e
chore: rename custom file with component descriptors & add there all CDs
kkafar Sep 13, 2022
ba5252b
fix: rename component descriptor file
kkafar Sep 13, 2022
de99905
chore: make main project use custom rnscreens.h file
kkafar Sep 13, 2022
449316f
chore: remove debug messages from cmakelists.txt
kkafar Sep 14, 2022
29bd651
chore: do not take rnscreens-generated.cpp source file
kkafar Sep 14, 2022
8e011e7
chore: add common/cpp as include directory for application project
kkafar Sep 14, 2022
81b86ad
chore: do not manually load rnscreens_modules shared library
kkafar Sep 14, 2022
4b56ca7
chore: remove all RNSScreen code that is & can be autogenerated by
kkafar Sep 14, 2022
05de5a5
fix: enforce non-local visibility of getDynamic symbol of
kkafar Sep 14, 2022
b573d77
style: remove codegend comments & format cpp code
kkafar Sep 14, 2022
7bd60c4
chore: remove unused common/cpp/Android.mk file
kkafar Sep 14, 2022
e2a4909
chore: update RNCLI configuration as common/cpp/Android.mk no longer
kkafar Sep 14, 2022
007895b
chore: generalise path resolution in CMake
kkafar Sep 15, 2022
c94c6e8
fix: iOS build
kkafar Sep 15, 2022
5d4912c
chore: move Android specific configuration & cpp files to Android
kkafar Sep 15, 2022
48630ec
fix: include path
kkafar Sep 15, 2022
2cfe611
chore: use JSI_EXPORT macro instead of setting compiler attribute
kkafar Sep 16, 2022
8ce7490
style: remove trailing whitespace
kkafar Sep 16, 2022
0d9f573
fix: add mising dot in glob pattern for cpp sources
kkafar Sep 16, 2022
213e592
style: use uppercase for CMake constants
kkafar Sep 16, 2022
9c00515
chore: parametrize library target name in cmake
kkafar Sep 16, 2022
3afe846
refact: generalize variable names
kkafar Sep 16, 2022
509f1af
Merge branch 'main' into @kkafar/fabric-autolinking
kkafar Sep 16, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 0 additions & 34 deletions android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -55,23 +55,6 @@ android {
versionCode 1
versionName "1.0"
buildConfigField "boolean", "IS_NEW_ARCHITECTURE_ENABLED", isNewArchitectureEnabled().toString()
if (isNewArchitectureEnabled()) {
var appProject = rootProject.allprojects.find {it.plugins.hasPlugin('com.android.application')}
externalNativeBuild {
ndkBuild {
arguments "APP_PLATFORM=android-21",
"APP_STL=c++_shared",
"NDK_TOOLCHAIN_VERSION=clang",
"GENERATED_SRC_DIR=${appProject.buildDir}/generated/source",
"PROJECT_BUILD_DIR=${appProject.buildDir}",
"REACT_ANDROID_DIR=${appProject.rootDir}/../node_modules/react-native/ReactAndroid",
"REACT_ANDROID_BUILD_DIR=${appProject.rootDir}/../node_modules/react-native/ReactAndroid/build"
cFlags "-Wall", "-Werror", "-fexceptions", "-frtti", "-DWITH_INSPECTOR=1"
cppFlags "-std=c++17"
targets "rnscreens_modules"
}
}
}
ndk {
abiFilters (*reactNativeArchitectures())
}
Expand All @@ -83,13 +66,6 @@ android {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}
if (isNewArchitectureEnabled()) {
externalNativeBuild {
ndkBuild {
path "src/main/jni/Android.mk"
}
}
}
packagingOptions {
// For some reason gradle only complains about the duplicated version of libreact_render libraries
// while there are more libraries copied in intermediates folder of the lib build directory, we exlude
Expand Down Expand Up @@ -139,13 +115,3 @@ dependencies {
implementation 'com.google.android.material:material:1.1.0'
implementation "androidx.core:core-ktx:1.5.0"
}

if (isNewArchitectureEnabled()) {
react {
reactRoot = rootProject.file("../node_modules/react-native/")
jsRootDir = file("../src/fabric/")
codegenDir = rootProject.file("../node_modules/react-native-codegen/")
libraryName = "rnscreens"
codegenJavaPackageName = "com.swmansion.rnscreens"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@

@DoNotStrip
public class RNScreensComponentsRegistry {
static {
SoLoader.loadLibrary("rnscreens_modules");
}

@DoNotStrip private final HybridData mHybridData;

Expand Down
13 changes: 2 additions & 11 deletions android/src/main/java/com/swmansion/rnscreens/RNScreensPackage.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,10 @@ import com.facebook.react.ReactPackage
import com.facebook.react.bridge.NativeModule
import com.facebook.react.bridge.ReactApplicationContext
import com.facebook.react.uimanager.ViewManager
import com.facebook.soloader.SoLoader

class RNScreensPackage : ReactPackage {
override fun createNativeModules(reactContext: ReactApplicationContext): List<NativeModule> {
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
// For Fabric, we load c++ native library here, this triggers screen's Fabric
// component registration which is necessary in order to avoid asking users
// to manually add init calls in their application code.
// This should no longer be needed if RN's autolink mechanism has Fabric support
SoLoader.loadLibrary("rnscreens_modules")
}
return emptyList()
}
override fun createNativeModules(reactContext: ReactApplicationContext): List<NativeModule> =
emptyList()

override fun createViewManagers(reactContext: ReactApplicationContext) =
listOf<ViewManager<*, *>>(
Expand Down
44 changes: 0 additions & 44 deletions android/src/main/jni/Android.mk

This file was deleted.

72 changes: 72 additions & 0 deletions android/src/main/jni/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
cmake_minimum_required(VERSION 3.13)
set(CMAKE_VERBOSE_MAKEFILE ON)

set(LIB_LITERAL rnscreens)
set(LIB_TARGET_NAME react_codegen_${LIB_LITERAL})

set(LIB_ANDROID_DIR ${CMAKE_CURRENT_SOURCE_DIR}/../../..)
set(LIB_COMMON_DIR ${LIB_ANDROID_DIR}/../common/cpp)
set(LIB_ANDROID_GENERATED_JNI_DIR ${LIB_ANDROID_DIR}/build/generated/source/codegen/jni)
set(LIB_ANDROID_GENERATED_COMPONENTS_DIR ${LIB_ANDROID_GENERATED_JNI_DIR}/react/renderer/components/${LIB_LITERAL})

add_compile_options(
-fexceptions
-frtti
-std=c++17
-Wall
-Wpedantic
-Wno-gnu-zero-variadic-macro-arguments
)

file(GLOB LIB_CUSTOM_SRCS CONFIGURE_DEPENDS *.cpp ${LIB_COMMON_DIR}/react/renderer/components/${LIB_LITERAL}/*.cpp)
file(GLOB LIB_CODEGEN_SRCS CONFIGURE_DEPENDS ${LIB_ANDROID_GENERATED_COMPONENTS_DIR}/*.cpp)

add_library(
${LIB_TARGET_NAME}
SHARED
${LIB_CUSTOM_SRCS}
${LIB_CODEGEN_SRCS}
)

target_include_directories(
${LIB_TARGET_NAME}
PUBLIC
.
${LIB_COMMON_DIR}
${LIB_ANDROID_GENERATED_JNI_DIR}
${LIB_ANDROID_GENERATED_COMPONENTS_DIR}
)

target_link_libraries(
${LIB_TARGET_NAME}
fbjni
folly_runtime
glog
jsi
react_codegen_rncore
react_debug
react_nativemodule_core
react_render_core
react_render_debug
react_render_graphics
react_render_mapbuffer
rrc_view
turbomodulejsijni
yoga
Copy link
Member

Choose a reason for hiding this comment

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

Based on the changes needed in rnsvg, maybe it would be good to add a comment that you should add here all libraries that your custom state depend on.

)

target_compile_options(
${LIB_TARGET_NAME}
PRIVATE
-DLOG_TAG=\"ReactNative\"
-fexceptions
-frtti
-std=c++17
-Wall
)

target_include_directories(
${CMAKE_PROJECT_NAME}
PUBLIC
${CMAKE_CURRENT_SOURCE_DIR}
)
Comment on lines +68 to +72
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 required, so when preprocessor expands #include <rnscreens.h> directive in autogenerated rncli.cpp file (its the place of adding CD providers to registry) it takes our custom rnscreens.h file (android/src/main/jni/rnscreens.h) instead of autogenerated one. This is crucial as it is the only way I found to inject RNSScreenComponentDescriptor symbol, so it is visible from the perspective of rncli.cpp translation unit.

9 changes: 0 additions & 9 deletions android/src/main/jni/OnLoad.cpp

This file was deleted.

66 changes: 0 additions & 66 deletions android/src/main/jni/RNScreensComponentsRegistry.cpp

This file was deleted.

34 changes: 0 additions & 34 deletions android/src/main/jni/RNScreensComponentsRegistry.h

This file was deleted.

12 changes: 12 additions & 0 deletions android/src/main/jni/rnscreens.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#include "rnscreens.h"
kkafar marked this conversation as resolved.
Show resolved Hide resolved

namespace facebook {
namespace react {


std::shared_ptr<TurboModule> rnscreens_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams &params) {
return nullptr;
}

} // namespace react
} // namespace facebook
15 changes: 15 additions & 0 deletions android/src/main/jni/rnscreens.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#pragma once

#include <ReactCommon/JavaTurboModule.h>
#include <ReactCommon/TurboModule.h>
#include <jsi/jsi.h>
#include <react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h>
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 include directive makes RNSScreenComponentDescriptor symbol visible in rncli.cpp.


namespace facebook {
namespace react {

JSI_EXPORT
tomekzaw marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to add information why it is needed at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

std::shared_ptr<TurboModule> rnscreens_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams &params);

} // namespace react
} // namespace facebook
37 changes: 0 additions & 37 deletions common/cpp/Android.mk

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
#include <react/renderer/components/rnscreens/EventEmitters.h>
#include <react/renderer/components/rnscreens/Props.h>
#include <react/renderer/components/view/ConcreteViewShadowNode.h>
#include <jsi/jsi.h>

namespace facebook {
namespace react {

extern const char RNSScreenComponentName[];
JSI_EXPORT extern const char RNSScreenComponentName[];

class RNSScreenShadowNode final : public ConcreteViewShadowNode<
class JSI_EXPORT RNSScreenShadowNode final : public ConcreteViewShadowNode<
tomekzaw marked this conversation as resolved.
Show resolved Hide resolved
RNSScreenComponentName,
RNSScreenProps,
RNSScreenEventEmitter,
Expand Down
Loading