-
Notifications
You must be signed in to change notification settings - Fork 12
feat: fabric support #167
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
feat: fabric support #167
Changes from 9 commits
05a9e2d
782f508
bedb1ca
73f8c8d
d4b9b51
fcb6068
100906b
0dc7107
3b92b24
0e995b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,21 @@ | ||
| val usercentricsVersion = "2.23.3" | ||
| val reactNativeVersion = "+" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does the "+" mean?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It't a common approach to use "+ " in sdk version, it's means that the host app manage what RN will be used
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
|
|
||
| fun BooleanProperty(name: String): Boolean { | ||
| return project.hasProperty(name) && project.property(name) == "true" | ||
| } | ||
|
|
||
| val isNewArchitectureEnabled = BooleanProperty("newArchEnabled") | ||
| val isHermesEnabled = BooleanProperty("hermesEnabled") | ||
|
|
||
| plugins { | ||
| id("com.android.library") | ||
| id("org.jetbrains.kotlin.android") | ||
| id("kotlin-android") | ||
| } | ||
|
|
||
| val usercentricsVersion = "2.23.3" | ||
| val reactNativeVersion = "0.79.6" | ||
| if (isNewArchitectureEnabled) { | ||
| apply(plugin = "com.facebook.react") | ||
| } | ||
|
|
||
| android { | ||
| namespace = "com.usercentrics.reactnative" | ||
|
|
@@ -16,15 +27,14 @@ android { | |
| testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" | ||
| consumerProguardFiles("proguard-rules.pro") | ||
|
|
||
| val isNewArchEnabled = project.findProperty("newArchEnabled") == "true" | ||
| val isHermesEnabled = project.findProperty("hermesEnabled") == "true" | ||
|
|
||
| buildConfigField("boolean", "IS_NEW_ARCHITECTURE_ENABLED", isNewArchEnabled.toString()) | ||
| // BuildConfig flags | ||
| buildConfigField("boolean", "IS_NEW_ARCHITECTURE_ENABLED", isNewArchitectureEnabled.toString()) | ||
| buildConfigField("boolean", "IS_HERMES_ENABLED", isHermesEnabled.toString()) | ||
|
|
||
| ndk { | ||
| abiFilters += listOf("armeabi-v7a", "arm64-v8a", "x86", "x86_64") | ||
| } | ||
|
|
||
| } | ||
|
|
||
| compileOptions { | ||
|
|
@@ -35,14 +45,12 @@ android { | |
| kotlinOptions { | ||
| jvmTarget = "17" | ||
| } | ||
|
|
||
| compileOptions { | ||
| sourceCompatibility = JavaVersion.VERSION_17 | ||
| targetCompatibility = JavaVersion.VERSION_17 | ||
| } | ||
|
|
||
| sourceSets { | ||
| getByName("main").java.srcDirs("src/main/kotlin") | ||
| getByName("main").java.srcDirs( | ||
| "src/main/kotlin", | ||
| "$buildDir/generated/codegen/NativeModules" | ||
| ) | ||
|
Comment on lines
+50
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify generated codegen path exists or is created before build. Adding Confirm:
🏁 Script executed: #!/bin/bash
# Search for codegen task or configuration that generates this directory
rg -i "codegen|NativeModules" --type=gradle --type=kts -A 3 -B 3Length of output: 27 🏁 Script executed: #!/bin/bash
# Search for codegen or NativeModules references in gradle files
rg -i "codegen|NativeModules" -A 3 -B 3 --type gradleLength of output: 1164 🏁 Script executed: #!/bin/bash
# Search all .gradle and .kts files for codegen/task definitions
fd -e gradle -e kts | head -20 | xargs rg -i "codegen|NativeModules|register.*Task" -A 2 -B 2Length of output: 1784 🏁 Script executed: #!/bin/bash
# Look for sourceSet configurations and task dependencies
rg "sourceSets\|tasks\s*{" --type gradle -A 5 -B 2Length of output: 105 🏁 Script executed: #!/bin/bash
# Check for isNewArchitectureEnabled conditions
rg "isNewArchitectureEnabled" -A 5 -B 2Length of output: 1249 🏁 Script executed: #!/bin/bash
# Get full context around lines 50-53 in android/build.gradle.kts
cat android/build.gradle.kts | head -80 | tail -40Length of output: 1061 🏁 Script executed: #!/bin/bash
# Look for all sourceSets and tasks blocks in the file
rg -n "sourceSets|tasks|if.*isNewArchitectureEnabled" android/build.gradle.kts -B 1 -A 8Length of output: 446 🏁 Script executed: #!/bin/bash
# Check the com.facebook.react plugin behavior - search in node_modules or docs
find . -name "*facebook*react*" -type f 2>/dev/null | head -10Length of output: 0 🌐 Web query: 💡 Result: The Codegen-generated Android files (including NativeModules/Turbo module Java/Kotlin and JNI glue) are written to your Gradle build's generated folder:
To produce them run the Gradle task from the android directory:
Or use the Codegen CLI:
Docs: React Native — Using Codegen (generated path + gradle task) and Codegen CLI. [1][2][3] Sources Fix incorrect codegen path and conditional sourceSets configuration. The generated codegen path and sourceSets configuration have two issues:
Fix: Move the sourceSets block inside the 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| buildFeatures { | ||
|
|
@@ -54,23 +62,33 @@ android { | |
| useLegacyPackaging = false | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| dependencies { | ||
| implementation("com.facebook.react:react-android:$reactNativeVersion") | ||
| implementation("com.usercentrics.sdk:usercentrics-ui:$usercentricsVersion") | ||
|
|
||
| // Unit tests (JVM) | ||
| // Unit tests | ||
| testImplementation("junit:junit:4.13.2") | ||
| testImplementation("io.mockk:mockk:1.13.12") | ||
| testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.9.0") | ||
| testImplementation("com.facebook.react:react-android:$reactNativeVersion") | ||
|
|
||
| // Instrumented tests (Android) | ||
| // Instrumented tests | ||
| androidTestImplementation("com.facebook.react:react-android:$reactNativeVersion") | ||
| androidTestImplementation("io.mockk:mockk-android:1.12.0") | ||
| androidTestImplementation("junit:junit:4.13.2") | ||
| androidTestImplementation("androidx.test.ext:junit-ktx:1.1.3") | ||
| androidTestImplementation("androidx.test:runner:1.4.0") | ||
| androidTestImplementation("androidx.test:rules:1.4.0") | ||
| } | ||
|
|
||
| // Fix for Gradle task dependency issue with React Native autolinking | ||
| // This ensures that packageReleaseResources task waits for generateReleaseResValues to complete | ||
| afterEvaluate { | ||
| if (project.tasks.findByName("packageReleaseResources") != null && project.tasks.findByName(":usercentrics_react-native-sdk:generateReleaseResValues") != null) { | ||
| project.tasks.named("packageReleaseResources").configure { | ||
| dependsOn(":usercentrics_react-native-sdk:generateReleaseResValues") | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,51 @@ | ||
| package com.usercentrics.reactnative | ||
|
|
||
| import com.facebook.react.ReactPackage | ||
| import com.facebook.react.TurboReactPackage | ||
| import com.facebook.react.bridge.NativeModule | ||
| import com.facebook.react.bridge.ReactApplicationContext | ||
| import com.facebook.react.module.annotations.ReactModuleList | ||
| import com.facebook.react.module.model.ReactModuleInfo | ||
| import com.facebook.react.module.model.ReactModuleInfoProvider | ||
| import com.facebook.react.uimanager.ViewManager | ||
| import com.facebook.react.turbomodule.core.interfaces.TurboModule | ||
| import com.usercentrics.reactnative.api.UsercentricsProxyImpl | ||
|
|
||
| class RNUsercentricsPackage : ReactPackage { | ||
| @ReactModuleList( | ||
| nativeModules = [ | ||
| RNUsercentricsModule::class, | ||
| ], | ||
| ) | ||
| class RNUsercentricsPackage : TurboReactPackage() { | ||
|
|
||
| private val usercentricsProxy = UsercentricsProxyImpl() | ||
|
|
||
| override fun createViewManagers(reactContext: ReactApplicationContext): MutableList<ViewManager<*, *>> { | ||
| return mutableListOf() | ||
| override fun getModule(s: String, reactApplicationContext: ReactApplicationContext): NativeModule? { | ||
| when (s) { | ||
| RNUsercentricsModule.NAME -> { | ||
| val reactContextProvider = ReactContextProviderImpl(reactApplicationContext) | ||
| return RNUsercentricsModule(reactApplicationContext, usercentricsProxy, reactContextProvider) | ||
| } | ||
| } | ||
| return null | ||
| } | ||
|
|
||
| override fun createNativeModules(reactContext: ReactApplicationContext): MutableList<NativeModule> { | ||
| val reactContextProvider = ReactContextProviderImpl(reactContext) | ||
| return mutableListOf(RNUsercentricsModule(reactContext, usercentricsProxy, reactContextProvider)) | ||
| override fun getReactModuleInfoProvider(): ReactModuleInfoProvider = | ||
| ReactModuleInfoProvider { | ||
| val moduleInfos: MutableMap<String, ReactModuleInfo> = HashMap() | ||
| val isTurboModule = BuildConfig.IS_NEW_ARCHITECTURE_ENABLED | ||
| moduleInfos[RNUsercentricsModule.NAME] = | ||
| ReactModuleInfo( | ||
| RNUsercentricsModule.NAME, | ||
| RNUsercentricsModule.NAME, | ||
| false, // canOverrideExistingModule | ||
| false, // needsEagerInit | ||
| true, // hasConstants | ||
| false, // isCxxModule | ||
| isTurboModule, | ||
| ) | ||
|
Comment on lines
+36
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix className in ReactModuleInfo. Second parameter must be the fully qualified class name, not the module name. Using the name twice can break legacy reflection paths. Apply: - ReactModuleInfo(
- RNUsercentricsModule.NAME,
- RNUsercentricsModule.NAME,
+ ReactModuleInfo(
+ RNUsercentricsModule.NAME,
+ RNUsercentricsModule::class.java.name,
false, // canOverrideExistingModule
false, // needsEagerInit
true, // hasConstants
false, // isCxxModule
isTurboModule,
)🤖 Prompt for AI Agents |
||
| moduleInfos | ||
| } | ||
|
|
||
| override fun createViewManagers(reactContext: ReactApplicationContext): List<ViewManager<*, *>> { | ||
| return emptyList() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,85 @@ | ||||||||||||||||||||||
| cmake_minimum_required(VERSION 3.13) | ||||||||||||||||||||||
| set(CMAKE_VERBOSE_MAKEFILE ON) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Set library name | ||||||||||||||||||||||
| set(LIB_TARGET_NAME react_codegen_RNUsercentricsModule) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Add compile options | ||||||||||||||||||||||
| add_compile_options( | ||||||||||||||||||||||
| -fexceptions | ||||||||||||||||||||||
| -frtti | ||||||||||||||||||||||
| -std=c++20 | ||||||||||||||||||||||
| -Wall | ||||||||||||||||||||||
| -Wpedantic | ||||||||||||||||||||||
| -Wno-gnu-zero-variadic-macro-arguments | ||||||||||||||||||||||
| -Wno-dollar-in-identifier-extension | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Find source files | ||||||||||||||||||||||
| file(GLOB LIB_CUSTOM_SRCS CONFIGURE_DEPENDS *.cpp) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Create the library | ||||||||||||||||||||||
| add_library( | ||||||||||||||||||||||
| ${LIB_TARGET_NAME} | ||||||||||||||||||||||
| SHARED | ||||||||||||||||||||||
| ${LIB_CUSTOM_SRCS} | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Include directories | ||||||||||||||||||||||
| target_include_directories( | ||||||||||||||||||||||
| ${LIB_TARGET_NAME} | ||||||||||||||||||||||
| PUBLIC | ||||||||||||||||||||||
| . | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Link libraries based on React Native version | ||||||||||||||||||||||
| if(ReactAndroid_VERSION_MINOR GREATER_EQUAL 76) | ||||||||||||||||||||||
| target_link_libraries( | ||||||||||||||||||||||
| ${LIB_TARGET_NAME} | ||||||||||||||||||||||
| ReactAndroid::reactnative | ||||||||||||||||||||||
| ReactAndroid::jsi | ||||||||||||||||||||||
| fbjni::fbjni | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| else() | ||||||||||||||||||||||
| 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 | ||||||||||||||||||||||
| react_render_componentregistry | ||||||||||||||||||||||
| react_utils | ||||||||||||||||||||||
| rrc_view | ||||||||||||||||||||||
| turbomodulejsijni | ||||||||||||||||||||||
| yoga | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| endif() | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Compile options based on React Native version | ||||||||||||||||||||||
| if(ReactAndroid_VERSION_MINOR GREATER_EQUAL 80) | ||||||||||||||||||||||
| target_compile_reactnative_options(${LIB_TARGET_NAME} PRIVATE) | ||||||||||||||||||||||
| else() | ||||||||||||||||||||||
| target_compile_options( | ||||||||||||||||||||||
| ${LIB_TARGET_NAME} | ||||||||||||||||||||||
| PRIVATE | ||||||||||||||||||||||
| -DLOG_TAG=\"ReactNative\" | ||||||||||||||||||||||
| -fexceptions | ||||||||||||||||||||||
| -frtti | ||||||||||||||||||||||
| -std=c++20 | ||||||||||||||||||||||
| -Wall | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| endif() | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Additional include directories | ||||||||||||||||||||||
| target_include_directories( | ||||||||||||||||||||||
| ${CMAKE_PROJECT_NAME} | ||||||||||||||||||||||
| PUBLIC | ||||||||||||||||||||||
| ${CMAKE_CURRENT_SOURCE_DIR} | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
Comment on lines
+81
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix incorrect target in target_include_directories. You’re setting include dirs on ${CMAKE_PROJECT_NAME}, which is not your library target and often isn’t a CMake target at all. Use ${LIB_TARGET_NAME}. Apply this diff: -target_include_directories(
- ${CMAKE_PROJECT_NAME}
- PUBLIC
- ${CMAKE_CURRENT_SOURCE_DIR}
-)
+target_include_directories(
+ ${LIB_TARGET_NAME}
+ PUBLIC
+ ${CMAKE_CURRENT_SOURCE_DIR}
+)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| #include <ReactCommon/JavaTurboModule.h> | ||
| #include <ReactCommon/TurboModule.h> | ||
| #include <jsi/jsi.h> | ||
|
|
||
| namespace facebook { | ||
| namespace react { | ||
|
|
||
| JSI_EXPORT | ||
| std::shared_ptr<TurboModule> RNUsercentricsModule_ModuleProvider( | ||
| const std::string &moduleName, | ||
| const JavaTurboModule::InitParams ¶ms) { | ||
| // This is a placeholder implementation | ||
| // The actual module is implemented in Java/Kotlin | ||
| return nullptr; | ||
| } | ||
|
|
||
| } // namespace react | ||
| } // namespace facebook | ||
|
|
||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| #include <jni.h> | ||
| #include <jsi/jsi.h> | ||
| #include <fbjni/fbjni.h> | ||
| #include <ReactCommon/CallInvokerHolder.h> | ||
| #include <ReactCommon/TurboModuleUtils.h> | ||
|
|
||
| namespace facebook { | ||
| namespace react { | ||
|
|
||
| // This is a minimal C++ TurboModule bridge that delegates to the Kotlin implementation | ||
| // The actual TurboModule implementation is in RNUsercentricsModule.kt | ||
|
|
||
| extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *) { | ||
| return facebook::jni::initialize(vm, [] { | ||
| // No additional initialization needed | ||
| // The Kotlin TurboModule will be loaded via TurboReactPackage | ||
| }); | ||
| } | ||
|
|
||
| } // namespace react | ||
| } // namespace facebook | ||
|
|
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.
🧩 Analysis chain
Pin React Native version for reproducible builds and library stability.
Using
"+"as a version specifier allows any version, which violates reproducible-build principles and can introduce breaking changes. This is especially critical for a library: consumers need stable, predictable transitive dependencies.Apply this diff to pin a specific version:
If you need flexibility across multiple versions, document the supported range in
README.mdand use a bounded range (e.g.,0.73.+or explicit range), or manage it viabuild.gradleinstead of hardcoding here.Please verify the intended React Native version(s) to support and confirm this is not a temporary placeholder.
I need to gather information to properly verify this review comment. Let me inspect the actual codebase and search for best practices.
🏁 Script executed:
Length of output: 2429
🌐 Web query:
💡 Result:
Short answer: avoid using the "+" (dynamic) version specifier for React Native Android dependencies in production. It can resolve to a newer incompatible artifact at build time, cause non-reproducible builds, and break RN native linking. Instead pin versions and use Gradle features to make upgrades deliberate and reproducible. [1][2][3][4]
Practical best practices
Quick examples
implementation "com.facebook.react:react-native:0.71.8". [4]
configurations.all {
resolutionStrategy.force("com.facebook.react:react-native:0.71.8")
}
(Use only if you understand side effects.) [3][4]
./gradlew dependencies --write-locks (see Gradle docs). [1]
If you want, tell me your React Native version and Gradle/AGP versions and I’ll propose an exact migration/lock strategy and a short patch you can apply.
I found the issue is real and valid. Let me check for context on what the intended React Native version should be:
🏁 Script executed:
Length of output: 756
📝 Committable suggestion
🤖 Prompt for AI Agents