test: code review #169
test: code review #169
Conversation
- Fabric/TurboModules configurados - STL compartilhada funcionando - fbjni 0.6.0 + Hermes + Flipper - Patches aplicados para react-native-screens e react-native-safe-area-context - CodeGen StringLiteralUnionTypeAnnotation ainda pendente
WalkthroughThis PR upgrades the React Native SDK to support the New Architecture (TurboModules/Fabric) across Android, iOS, and JavaScript layers. Changes include creating module specs for TurboModule wiring, migrating to Gradle version catalogs, refactoring example app structure with JNI/C++ support, modernizing build tooling (Gradle 8.x, SDK levels), and adding automation scripts for environment setup and validation. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JavaScript Layer
participant TM as TurboModule<br/>(New Arch)
participant Bridge as Legacy Bridge<br/>(Old Arch)
participant Android as Android Module<br/>(RNUsercentricsModule)
participant iOS as iOS Module<br/>(RNUsercentricsModule)
alt New Architecture Enabled
JS->>TM: call configure(options)
TM->>Android: JNI call
TM->>iOS: invoke method
Android->>Android: RNUsercentricsModuleSpec override
iOS->>iOS: NativeUsercentricsSpec method
else Old Architecture (Fallback)
JS->>Bridge: NativeModules call
Bridge->>Android: invoke ReactMethod
Bridge->>iOS: invoke RCT method
end
Android->>Android: Execute business logic
iOS->>iOS: Execute business logic
sequenceDiagram
participant User as Developer
participant Setup as auto-setup.sh
participant Check as check-requirements.sh
participant Env as Environment
User->>Setup: npm run auto-setup
Setup->>Env: Detect OS (macOS/Linux)
Setup->>Env: Install Homebrew (macOS)
Setup->>Env: Install/verify Node.js ≥18
Setup->>Env: Install/verify Java 17+
Setup->>Env: Configure Android SDK
Setup->>Env: Install iOS tools (macOS)
Setup->>Env: Install npm dependencies
Setup->>Check: Run requirements check
Check->>Env: Validate all tools
Check-->>User: Summary report (PASS/FAIL/WARN)
Setup-->>User: Setup complete + next steps
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Auto review disabled due to large PR. If you still want me to review this PR? Please comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
example/android/gradlew (1)
89-89: Remove duplicate DEFAULT_JVM_OPTS definition.
DEFAULT_JVM_OPTSis defined twice—once at line 89 and again at lines 204-206 with identical values. The first definition is overridden before use, serving no purpose.Apply this diff to remove the redundant first definition:
-# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. -DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' - # Use the maximum available, or set MAX_FD != -1 to use that value. MAX_FD=maximumAlso applies to: 203-206
🧹 Nitpick comments (11)
example/index.js (1)
8-9: Consider reverting to importing from app.json to maintain single source of truth.Hardcoding the app name duplicates the value from
app.json(line 2), which previously served as the source. If these fall out of sync,AppRegistry.registerComponentmay fail to register correctly.To restore the single source of truth, consider reverting to the import approach:
-const appName = 'usercentrics' - +import {name as appName} from './app.json' + AppRegistry.registerComponent(appName, () => App)Alternatively, if you prefer the hardcoded approach, add a verification script to ensure both values stay in sync:
#!/bin/bash # Description: Verify app name consistency between app.json and index.js APP_JSON_NAME=$(jq -r '.name' example/app.json) INDEX_JS_NAME=$(rg -oP "const appName = '(.+)'" example/index.js | cut -d"'" -f2) if [ "$APP_JSON_NAME" != "$INDEX_JS_NAME" ]; then echo "Error: App name mismatch between app.json ($APP_JSON_NAME) and index.js ($INDEX_JS_NAME)" exit 1 fi echo "App names match: $APP_JSON_NAME"example/src/screens/WebviewIntegrationScreen.tsx (1)
2-2: Remove unused import.The
Textimport is not used anywhere in this component.Apply this diff to remove the unused import:
-import { StyleSheet, View, Text } from 'react-native'; +import { StyleSheet, View } from 'react-native';scripts/run-android.sh (2)
5-6: Consider handling systems wherelsofis unavailable.The
lsofcommand may not be available on all systems (e.g., minimal Linux containers, Windows Git Bash). While the|| trueprevents script failure, the ports won't be cleaned up.Consider adding a check:
# Kill processes on ports 8081 and 7007 -lsof -i tcp:8081 | awk 'NR!=1 {print $2}' | xargs kill 2>/dev/null || true -lsof -i tcp:7007 | awk 'NR!=1 {print $2}' | xargs kill 2>/dev/null || true +if command -v lsof &> /dev/null; then + lsof -i tcp:8081 | awk 'NR!=1 {print $2}' | xargs kill 2>/dev/null || true + lsof -i tcp:7007 | awk 'NR!=1 {print $2}' | xargs kill 2>/dev/null || true +else + echo "⚠️ lsof not available, skipping port cleanup" +fi
22-23: Consider adding a check for React Native CLI availability.While developers should have
react-nativeinstalled, adding a check provides clearer error messaging if it's missing.echo "🤖 Starting Android build with Metro bundler..." +if ! command -v react-native &> /dev/null; then + echo "❌ react-native CLI not found. Please install it first." + exit 1 +fi react-native run-androidscripts/check-requirements.sh (2)
53-67: Version comparison may fail for patch versions.The version comparison only extracts major.minor and ignores patch versions. This could lead to incorrect comparisons when patch versions matter.
For example, versions like
18.0.0and18.0.1would both extract as18.0, which is fine for the current requirements. However, if you ever need patch-level precision, consider enhancing the logic:# Enhanced version comparison supporting major.minor.patch check_version() { local current=$1 local required=$2 local name=$3 if [ -z "$current" ]; then print_status "FAIL" "$name not found" "Please install $name" return 1 fi # Extract version components IFS='.' read -r curr_major curr_minor curr_patch <<< "$current" IFS='.' read -r req_major req_minor req_patch <<< "$required" # Set defaults for missing components curr_minor=${curr_minor:-0} curr_patch=${curr_patch:-0} req_minor=${req_minor:-0} req_patch=${req_patch:-0} # Compare versions if [ "$curr_major" -gt "$req_major" ] || \ ([ "$curr_major" -eq "$req_major" ] && [ "$curr_minor" -gt "$req_minor" ]) || \ ([ "$curr_major" -eq "$req_major" ] && [ "$curr_minor" -eq "$req_minor" ] && [ "$curr_patch" -ge "$req_patch" ]); then print_status "PASS" "$name version $current" "Required: >= $required" return 0 else print_status "FAIL" "$name version $current is too old" "Required: >= $required" return 1 fi }
291-297: Verify patch application with version-agnostic checks.The patch verification checks for a specific string in a specific file, which is fragile. If the patch changes or the package version changes, this check could produce false positives or negatives.
Consider checking for a marker file or using a more reliable verification method:
# Check if patches are applied if [ -d "$PROJECT_ROOT/example/node_modules/react-native-screens" ]; then # Check for patch marker or specific patch effect PATCH_MARKER="$PROJECT_ROOT/example/node_modules/.patches-applied" if [ -f "$PATCH_MARKER" ] && grep -q "react-native-screens" "$PATCH_MARKER"; then print_status "PASS" "Patches applied successfully" "react-native-screens patched" else print_status "WARN" "Patches may not be applied" "Run: npm run postinstall in example" fi fiscripts/auto-setup.sh (3)
204-221: JAVA_HOME configuration may not persist correctly.The script appends JAVA_HOME exports to
~/.zshrc, but this assumes the user is using Zsh. Users on different shells (bash, fish, etc.) won't have these changes applied.Consider detecting the user's shell and updating the appropriate rc file:
# Set JAVA_HOME if not set if [ -z "$JAVA_HOME" ]; then JAVA_HOME_VALUE="" if [ "$OS" = "macos" ] && [ -d "/usr/local/opt/openjdk@17" ]; then JAVA_HOME_VALUE="/usr/local/opt/openjdk@17" elif command_exists sdk; then JAVA_HOME_VALUE=$(sdk home java 17.0.2-zulu 2>/dev/null || echo "") fi if [ ! -z "$JAVA_HOME_VALUE" ]; then export JAVA_HOME="$JAVA_HOME_VALUE" # Detect and update appropriate shell RC file if [ -n "$BASH_VERSION" ]; then echo "export JAVA_HOME=\"$JAVA_HOME_VALUE\"" >> ~/.bashrc elif [ -n "$ZSH_VERSION" ]; then echo "export JAVA_HOME=\"$JAVA_HOME_VALUE\"" >> ~/.zshrc fi print_status "SUCCESS" "JAVA_HOME set to $JAVA_HOME_VALUE" print_status "INFO" "Restart your terminal to load the new environment variable" else print_status "INFO" "Please set JAVA_HOME manually" fi else print_status "SKIP" "JAVA_HOME already set" "$JAVA_HOME" fi
232-239: Android SDK PATH configuration assumes Zsh.Similar to the JAVA_HOME issue, the PATH export is appended to
~/.zshrcwhich won't work for users on other shells.Apply shell detection as suggested in the previous comment for JAVA_HOME.
418-420: Potential silent failure when cleaning caches.The script redirects all output from
clean-all-caches.shto/dev/null, which could hide important error messages or warnings.Consider capturing output and only suppressing it on success:
- ./scripts/clean-all-caches.sh >/dev/null 2>&1 && \ - print_status "SUCCESS" "Caches cleaned" || \ - print_status "FAIL" "Failed to clean caches" + CLEAN_OUTPUT=$(./scripts/clean-all-caches.sh 2>&1) + if [ $? -eq 0 ]; then + print_status "SUCCESS" "Caches cleaned" + else + print_status "FAIL" "Failed to clean caches" "$CLEAN_OUTPUT" + fisrc/Usercentrics.tsx (1)
1-1: Unused import.The
Platformimport is not used anywhere in this file. Consider removing it to keep the imports clean.Apply this diff to remove the unused import:
-import {NativeModules, Platform} from 'react-native'; +import {NativeModules} from 'react-native';example/metro.config.js (1)
4-38: Avoid resolving the default Metro config twice.
getDefaultConfig(__dirname)is invoked once insidecustomConfigand again in the export. That duplicates the I/O and work Metro already did. Passing the already-fetched config intocustomConfigkeeps the behaviour identical while avoiding the extra lookup.-const customConfig = async () => { - const { - resolver: { sourceExts, assetExts }, - } = await getDefaultConfig(__dirname); +const customConfig = defaultConfig => { + const { + resolver: { sourceExts, assetExts }, + } = defaultConfig; @@ - return { + return { transformer: { getTransformOptions: async () => ({ @@ - watchFolders: [ + watchFolders: [ path.resolve(__dirname, 'src'), path.resolve(__dirname, '..'), ], }; }; module.exports = (async () => { const defaultConfig = await getDefaultConfig(__dirname); - const config = await customConfig(); + const config = customConfig(defaultConfig); return mergeConfig(defaultConfig, config); })();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
example/ios/Podfile.lockis excluded by!**/*.lockexample/package-lock.jsonis excluded by!**/package-lock.jsonexample/yarn.lockis excluded by!**/yarn.lock,!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (45)
README.md(2 hunks)android/build.gradle(3 hunks)android/gradle/wrapper/gradle-wrapper.properties(1 hunks)android/settings.gradle(1 hunks)android/src/main/AndroidManifest.xml(1 hunks)android/src/main/java/com/usercentrics/reactnativeusercentrics/RNUsercentricsModule.kt(14 hunks)android/src/main/java/com/usercentrics/reactnativeusercentrics/RNUsercentricsModuleSpec.kt(1 hunks)android/src/main/java/com/usercentrics/reactnativeusercentrics/RNUsercentricsPackage.kt(1 hunks)babel.config.js(1 hunks)example/android/app/build.gradle(1 hunks)example/android/app/src/debug/java/com/usercentrics/reactnativesdk/example/ReactNativeFlipper.kt(2 hunks)example/android/app/src/main/AndroidManifest.xml(1 hunks)example/android/app/src/main/java/com/example/MainActivity.kt(0 hunks)example/android/app/src/main/java/com/usercentrics/reactnativesdk/example/MainActivity.kt(1 hunks)example/android/app/src/main/java/com/usercentrics/reactnativesdk/example/MainApplication.kt(2 hunks)example/android/app/src/main/jni/CMakeLists.txt(1 hunks)example/android/app/src/main/jni/OnLoad.cpp(1 hunks)example/android/app/src/main/jni/rncli.cpp(1 hunks)example/android/app/src/main/jni/rncli.h(1 hunks)example/android/build.gradle(2 hunks)example/android/gradle.properties(1 hunks)example/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)example/android/gradlew(2 hunks)example/android/settings.gradle(1 hunks)example/app.json(1 hunks)example/babel.config.js(1 hunks)example/generateAutolinking.js(1 hunks)example/index.js(1 hunks)example/ios/example/AppDelegate.swift(2 hunks)example/metro.config.js(1 hunks)example/package.json(1 hunks)example/patches/react-native-screens+3.37.0.patch(1 hunks)example/react-native.config.js(1 hunks)example/src/screens/WebviewIntegrationScreen.tsx(2 hunks)gradle/libs.versions.toml(1 hunks)ios/RNUsercentricsModule.swift(2 hunks)ios/RNUsercentricsModuleSpec.h(1 hunks)package.json(2 hunks)scripts/auto-setup.sh(1 hunks)scripts/check-requirements.sh(1 hunks)scripts/clean-all-caches.sh(1 hunks)scripts/install-dependencies.sh(1 hunks)scripts/run-android.sh(1 hunks)src/NativeUsercentrics.ts(1 hunks)src/Usercentrics.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- example/android/app/src/main/java/com/example/MainActivity.kt
🧰 Additional context used
🧬 Code graph analysis (8)
scripts/check-requirements.sh (1)
scripts/auto-setup.sh (2)
print_status(24-46)check_version(54-74)
src/NativeUsercentrics.ts (9)
src/models/UsercentricsOptions.tsx (1)
UsercentricsOptions(3-45)src/models/BannerSettings.tsx (1)
BannerSettings(104-117)src/models/UsercentricsConsentUserResponse.tsx (1)
UsercentricsConsentUserResponse(3-14)src/models/UsercentricsServiceConsent.tsx (1)
UsercentricsServiceConsent(3-24)src/models/UserDecision.tsx (1)
UserDecision(1-10)src/models/TCFUserDecisions.tsx (1)
TCFUserDecisions(1-24)android/src/main/java/com/usercentrics/reactnativeusercentrics/RNUsercentricsModule.kt (1)
saveDecisions(187-194)android/src/main/java/com/usercentrics/reactnativeusercentrics/RNUsercentricsModuleSpec.kt (1)
saveDecisions(78-79)ios/RNUsercentricsModule.swift (1)
saveDecisions(182-188)
scripts/auto-setup.sh (1)
scripts/check-requirements.sh (2)
print_status(21-40)check_version(43-67)
example/android/app/src/main/jni/rncli.h (1)
example/android/app/src/main/jni/rncli.cpp (6)
rncli_ModuleProvider(17-20)rncli_ModuleProvider(17-17)rncli_cxxModuleProvider(22-25)rncli_cxxModuleProvider(22-22)rncli_registerProviders(27-31)rncli_registerProviders(27-27)
src/Usercentrics.tsx (3)
src/models/UsercentricsServiceConsent.tsx (1)
UsercentricsServiceConsent(3-24)src/models/UserDecision.tsx (1)
UserDecision(1-10)src/models/TCFUserDecisions.tsx (1)
TCFUserDecisions(1-24)
example/android/app/src/main/java/com/usercentrics/reactnativesdk/example/MainApplication.kt (1)
example/android/app/src/debug/java/com/usercentrics/reactnativesdk/example/ReactNativeFlipper.kt (1)
initializeFlipper(18-35)
ios/RNUsercentricsModuleSpec.h (1)
ios/RNUsercentricsModule.swift (25)
configure(24-33)isReady(35-45)showFirstLayer(47-60)showSecondLayer(62-75)restoreUserSession(85-91)getControllerId(93-95)clearUserSession(203-209)getConsents(97-99)getCMPData(101-103)getAdditionalConsentModeData(123-125)getTCFData(105-109)getUserSessionData(111-113)getUSPData(115-117)getABTestingVariant(119-121)setCMPId(77-79)setABTestingVariant(81-83)changeLanguage(127-133)acceptAll(144-149)acceptAllForTCF(135-142)denyAll(159-164)denyAllForTCF(151-157)saveDecisions(182-188)saveDecisionsForTCF(166-180)saveOptOutForCCPA(190-196)track(198-201)
example/ios/example/AppDelegate.swift (1)
ios/RNUsercentricsModule.swift (1)
moduleName(16-18)
🪛 Clang (14.0.6)
example/android/app/src/main/jni/OnLoad.cpp
[error] 1-1: 'jni.h' file not found
(clang-diagnostic-error)
example/android/app/src/main/jni/rncli.h
[error] 11-11: 'ReactCommon/CallInvoker.h' file not found
(clang-diagnostic-error)
ios/RNUsercentricsModuleSpec.h
[error] 1-1: 'React/RCTBridgeModule.h' file not found
(clang-diagnostic-error)
🪛 Shellcheck (0.11.0)
scripts/clean-all-caches.sh
[warning] 43-43: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 57-57: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 75-75: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 95-95: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 99-99: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 104-104: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
scripts/install-dependencies.sh
[warning] 32-32: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 42-42: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 51-51: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (39)
example/app.json (1)
2-3: LGTM! Ensure consistency with index.js.The app rename looks good. Note that the
namevalue must match the hardcodedappNameconstant inexample/index.jsfor the app to register correctly.example/android/gradlew (2)
136-142: LGTM!The restructured Java availability check using
command -vis POSIX-compliant and provides clear error handling whenjavais not found in PATH.
150-150: LGTM!The
shellcheck disable=SC3045comments appropriately suppress warnings aboutulimitusage in POSIX shells. The script already includes fallback handling with|| warnto gracefully handle platforms whereulimitmay not work as expected.Also applies to: 158-158
example/android/app/src/main/AndroidManifest.xml (1)
1-1: LGTM! Standard Android modernization.Removing the package attribute from the manifest aligns with modern Android build practices where the package/namespace is defined in the Gradle build files instead.
example/android/gradle/wrapper/gradle-wrapper.properties (1)
3-5: LGTM! Gradle upgrade with security enhancement.The upgrade to Gradle 8.9 and the addition of
validateDistributionUrl=truealign with the broader build system modernization and add security validation for the distribution URL.babel.config.js (1)
2-2: LGTM! Correct Babel preset update.The update to
@react-native/babel-presetaligns with React Native's modern package naming conventions and supports the New Architecture features.example/babel.config.js (1)
2-2: LGTM! Consistent Babel preset update.The preset update matches the change in the root
babel.config.js, ensuring consistency across the project.example/android/gradle.properties (2)
29-40: LGTM! New Architecture enablement configured correctly.The FLIPPER_VERSION upgrade,
newArchEnabled=true, andhermesEnabled=trueflags correctly enable TurboModules and Fabric support, aligning with the PR's objective to support the New Architecture.
42-45: Verify the architecture limitation is intentional.The
reactNativeArchitectures=arm64-v8asetting restricts builds to 64-bit ARM only, which excludes:
- x86_64 emulators (may require Rosetta/translation or ARM-based emulators)
- 32-bit ARM devices (armeabi-v7a)
Confirm this is intentional for your use case. If developers use x86_64 emulators, consider adding
x86_64to the architectures list for development builds.example/ios/example/AppDelegate.swift (2)
29-31: LGTM! Simplified bundle URL configuration.Removing the explicit
fallbackResource: nilparameter is a good simplification sincenilis the default value.
10-10: No action required—module name is already consistent.The JavaScript entry point correctly registers the component with
'usercentrics', matching the module name in the Swift code. The concern raised in the review comment has already been addressed in the codebase.example/generateAutolinking.js (1)
25-35: LGTM! Good validation and file writing.The config validation and file writing logic are well-implemented with appropriate error handling and user feedback.
scripts/run-android.sh (1)
8-20: LGTM! Directory detection handles expected scenarios.The directory detection logic appropriately handles the three expected launch contexts (project root, scripts folder, example folder) with clear error handling for unexpected cases.
README.md (2)
25-54: Comprehensive requirements documentation.The expanded requirements section provides clear, structured information about platform requirements, development environment prerequisites, and tooling versions. This will help developers quickly assess whether their environment meets the project's needs.
68-151: Excellent addition of automated setup workflows.The Quick Start section now provides both automated and manual setup paths with clear instructions. The troubleshooting section and development scripts table make it easy for developers to resolve issues and understand available tooling.
example/react-native.config.js (1)
1-22: Well-structured autolinking configuration.The React Native config properly defines the project structure and dependency details for both Android and iOS platforms. The use of
path.resolve()ensures correct path resolution, and the manual linking configuration will help with TurboModule integration.scripts/check-requirements.sh (1)
1-424: Excellent comprehensive environment validation.This script provides thorough environment checking with clear, actionable feedback. The color-coded output, detailed status reporting, and helpful next steps make it very user-friendly.
scripts/auto-setup.sh (2)
5-5: Good use of strict error handling.The
set -eflag ensures the script exits on any command failure, includingcdcommands. This provides robust error handling throughout the script without needing individual|| exitchecks on every command.
1-484: Comprehensive automated setup implementation.This script provides an impressive automated setup experience, handling multiple platforms and tools. The structured status reporting, error handling with
set -e, and clear next steps make it highly user-friendly.package.json (2)
36-41: New automation scripts enhance developer experience.The addition of these convenience scripts provides easy access to the comprehensive automation tooling added in this PR.
44-60: DevDependency version updates align with React Native 0.74.3.The version updates for Babel, TypeScript, Jest, and React packages are consistent with supporting React Native 0.74.3+ as mentioned in the README requirements.
example/patches/react-native-screens+3.37.0.patch (1)
84-85: MinSdk update aligns with project requirements.Updating minSdkVersion from 21 to 23 is consistent with the project's stated requirements in the README (Android 5.0/API 21+) and ensures compatibility. Note that line 94 also sets it to 23, which provides consistency.
src/NativeUsercentrics.ts (3)
1-15: LGTM! Clean TurboModule setup.The imports are well-organized and include all necessary types for the TurboModule spec interface.
17-62: Enum parameter mapping verified—no issues found.The verification confirms that
consentType: numberin the TypeScript interface correctly maps to native types across all platforms: Android usesInt, iOS usesNSInteger(SwiftInt), and both marshal properly from JavaScript numbers through the TurboModule bridge.
64-64: Module name consistency verified across all platforms.The export correctly uses
getEnforcing<Spec>('RNUsercentricsModule'), which matches the NAME constants in Android native modules and the moduleName() return value in iOS ("RNUsercentricsModule").src/Usercentrics.tsx (2)
18-21: Excellent TurboModule fallback pattern.The conditional binding with
NativeUsercentrics || NativeModules.RNUsercentricsModuleprovides a clean fallback mechanism for gradual TurboModule adoption. The comment clearly explains the intent.
58-58: Correct return type alignment with TurboModule spec.The return types have been properly updated from
Promise<[T]>(tuple) toPromise<Array<T>>, which is the correct type for TurboModule array marshaling. This ensures consistency with the TurboModule spec defined insrc/NativeUsercentrics.ts.Also applies to: 93-93, 98-98, 103-103, 108-108, 113-113, 118-118, 123-123
android/src/main/java/com/usercentrics/reactnativeusercentrics/RNUsercentricsPackage.kt (1)
7-7: LGTM! TurboModule import added for compatibility.The import addition prepares the package for TurboModule support, aligning with the broader architectural migration in this PR.
android/gradle/wrapper/gradle-wrapper.properties (1)
3-3: Gradle 8.7 upgrade necessary for modern Android tooling.The upgrade from Gradle 7.6 to 8.7 and the switch from
-binto-alldistribution are appropriate for:
- Version catalog support
- AGP 8.x compatibility
- TurboModule build requirements
The
-alldistribution includes sources and documentation, which is helpful for development.example/android/app/src/main/jni/OnLoad.cpp (1)
1-10: Standard JNI entry point for TurboModule support.The implementation follows React Native's standard pattern for JNI initialization using fbjni. The empty lambda provides a hook for registering additional JNI bindings when needed.
Note on static analysis warning: The Clang error about
jni.hfile not found is expected in this context, as these headers are provided by the Android NDK at build time and won't be present during static analysis outside the build environment.example/android/app/src/debug/java/com/usercentrics/reactnativesdk/example/ReactNativeFlipper.kt (3)
1-1: Package name updated for consistency.The package name change aligns with the example app structure.
12-12: ReactFlipperPlugin addition improves debugging capabilities.Adding
ReactFlipperPluginenables React DevTools integration in Flipper, which is standard for React Native 0.75 debugging setup.Also applies to: 23-23
28-32: Improved formatting and removed Fresco dependency.The multi-line lambda formatting improves readability. The removal of
FrescoFlipperPluginis appropriate if the app no longer uses Fresco for image loading (React Native moved away from Fresco as a hard dependency in newer versions).example/package.json (2)
12-19: All referenced scripts verified to exist.Verification confirms all shell scripts in
../scripts/and thegenerateAutolinking.jsfile are present and accounted for. No missing files detected.
22-29: Dependency upgrades verified as secure for TurboModule migration.The dependency updates are appropriate for the TurboModule migration to React Native 0.75. Security advisory checks confirm that both React 18.2.0 and React Native 0.75.0 are not affected by known high-severity vulnerabilities. The addition of Hermes engine, Babel transform plugins, and updated navigation packages are all necessary components for the new architecture.
android/src/main/AndroidManifest.xml (1)
1-1: Namespace properly defined in build.gradle — change is correct.The
packageattribute removal from the manifest is appropriate for AGP 7.0+. The namespace is correctly defined inandroid/build.gradleat line 37 asnamespace 'com.usercentrics.reactnative', confirming this modernization aligns with the Gradle upgrade.android/settings.gradle (1)
3-17: Centralized repository setup looks goodRepository and plugin management align with the new architecture template and resolve React Native artifacts correctly.
example/android/app/src/main/java/com/usercentrics/reactnativesdk/example/MainApplication.kt (1)
13-47: Host migration aligns with RN new architectureSwitching to
DefaultReactNativeHost, guarding the new-architecture entry point, and keeping Flipper behind the DEBUG flag all look correct.example/android/app/src/main/jni/rncli.h (1)
20-22: Generated TurboModule stubs look correctThe declarations match the RN CLI defaults, so native providers can be filled in later as needed.
| kotlinOptions { | ||
| jvmTarget = JavaVersion.VERSION_17 | ||
| } |
There was a problem hiding this comment.
Fix Kotlin JVM target assignment
kotlinOptions.jvmTarget expects a numeric string such as "17", but JavaVersion.VERSION_17 stringifies to "VERSION_17", which the Kotlin compiler rejects. Switch to the literal value or reuse the catalog entry so the module compiles.
kotlinOptions {
- jvmTarget = JavaVersion.VERSION_17
+ jvmTarget = libs.versions.jvm.target.get()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| kotlinOptions { | |
| jvmTarget = JavaVersion.VERSION_17 | |
| } | |
| kotlinOptions { | |
| jvmTarget = libs.versions.jvm.target.get() | |
| } |
🤖 Prompt for AI Agents
In example/android/app/build.gradle around lines 19 to 21, the
kotlinOptions.jvmTarget is set to JavaVersion.VERSION_17 which stringifies to
"VERSION_17" and is invalid for the Kotlin compiler; change the assignment to a
numeric string like "17" or reference the existing catalog/value that resolves
to "17" so kotlinOptions.jvmTarget receives the correct JVM target string.
| android { | ||
| compileSdkVersion = 34 | ||
| buildToolsVersion = "34.0.0" | ||
| } |
There was a problem hiding this comment.
Fix compileSdk assignment before merging
compileSdkVersion = 34 invokes a setter that AGP 8.x does not expose, so every Android subproject will fail to sync/build with No signature of method: BaseExtension.setCompileSdkVersion(). Use the supported DSL (compileSdk 34 or compileSdkVersion(34)) when you override the value in afterEvaluate.
Apply this diff to correct the block:
- android {
- compileSdkVersion = 34
- buildToolsVersion = "34.0.0"
- }
+ android {
+ compileSdk = 34
+ buildToolsVersion = "34.0.0"
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| android { | |
| compileSdkVersion = 34 | |
| buildToolsVersion = "34.0.0" | |
| } | |
| android { | |
| compileSdk = 34 | |
| buildToolsVersion = "34.0.0" | |
| } |
🤖 Prompt for AI Agents
In example/android/build.gradle around lines 48 to 51, the assignment
"compileSdkVersion = 34" uses a setter not exposed by AGP 8.x and causes
sync/build failures; replace it with the supported DSL by setting compileSdk 34
(or calling compileSdkVersion(34)) instead, keeping buildToolsVersion as-is and
applying this change where you override values in afterEvaluate so subprojects
no longer call the unsupported setter.
| if (result.error) { | ||
| console.error("❌ Error running react-native config", result.error); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
Add exit code validation.
The script only checks result.error but doesn't validate result.status. A non-zero exit code would be missed if the spawn itself succeeded but the command failed.
Apply this diff:
-if (result.error) {
+if (result.error || result.status !== 0) {
console.error("❌ Error running react-native config", result.error);
+ if (result.stderr) {
+ console.error("stderr:", result.stderr);
+ }
process.exit(1);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (result.error) { | |
| console.error("❌ Error running react-native config", result.error); | |
| process.exit(1); | |
| } | |
| if (result.error || result.status !== 0) { | |
| console.error("❌ Error running react-native config", result.error); | |
| if (result.stderr) { | |
| console.error("stderr:", result.stderr); | |
| } | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In example/generateAutolinking.js around lines 18 to 21, the script only checks
result.error and misses non-zero exit codes; update the check to validate
result.status as well (treat null/undefined or any non-zero value as failure),
log a descriptive error including both result.error and result.status/exit code,
and call process.exit with the non-zero exit code (or 1 if status is
unavailable) so failures from the spawned command are correctly surfaced.
| process.exit(1); | ||
| } | ||
|
|
||
| const config = JSON.parse(result.stdout); |
There was a problem hiding this comment.
Add error handling for JSON parsing.
JSON.parse can throw if the output is malformed. Wrap it in a try-catch block to provide a clear error message.
Apply this diff:
-const config = JSON.parse(result.stdout);
+let config;
+try {
+ config = JSON.parse(result.stdout);
+} catch (error) {
+ console.error("❌ Failed to parse react-native config output");
+ console.error("Output was:", result.stdout);
+ process.exit(1);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const config = JSON.parse(result.stdout); | |
| let config; | |
| try { | |
| config = JSON.parse(result.stdout); | |
| } catch (error) { | |
| console.error("❌ Failed to parse react-native config output"); | |
| console.error("Output was:", result.stdout); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In example/generateAutolinking.js around line 23, JSON.parse(result.stdout) can
throw on malformed JSON; wrap the parse in a try-catch that catches the
SyntaxError, logs or throws a new Error with a clear message including the
original error message and the raw result.stdout for debugging, and then rethrow
or exit so the caller knows parsing failed.
| transformer: { | ||
| getTransformOptions: async () => ({ | ||
| transform: { | ||
| experimentalImportSupport: false, | ||
| inlineRequires: true, | ||
| }, | ||
| }), | ||
| }, | ||
| blockList: exclusionList([ | ||
| new RegExp(`${moduleRoot}/node_modules/react/.*`), | ||
| new RegExp(`${moduleRoot}/node_modules/react-native/.*`), | ||
| ]), | ||
| }, | ||
| transformer: { | ||
| getTransformOptions: async () => ({ | ||
| transform: { | ||
| experimentalImportSupport: false, | ||
| inlineRequires: true, | ||
| resolver: { | ||
| assetExts: newAssetExts, | ||
| sourceExts: customSourceExts, | ||
| extraNodeModules: { |
There was a problem hiding this comment.
Add the SVG transformer before treating .svg files as source.
By removing .svg from assetExts and adding it to sourceExts, Metro now sends SVG files through the JavaScript transformer. Without wiring a dedicated babelTransformerPath (for example react-native-svg-transformer), any import './icon.svg' will fail because the default transformer cannot parse SVG XML. Please hook up the SVG transformer alongside this change so the pipeline keeps working.
return {
transformer: {
+ babelTransformerPath: require.resolve('react-native-svg-transformer'),
getTransformOptions: async () => ({
transform: {
experimentalImportSupport: false,
inlineRequires: true,
},
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| transformer: { | |
| getTransformOptions: async () => ({ | |
| transform: { | |
| experimentalImportSupport: false, | |
| inlineRequires: true, | |
| }, | |
| }), | |
| }, | |
| blockList: exclusionList([ | |
| new RegExp(`${moduleRoot}/node_modules/react/.*`), | |
| new RegExp(`${moduleRoot}/node_modules/react-native/.*`), | |
| ]), | |
| }, | |
| transformer: { | |
| getTransformOptions: async () => ({ | |
| transform: { | |
| experimentalImportSupport: false, | |
| inlineRequires: true, | |
| resolver: { | |
| assetExts: newAssetExts, | |
| sourceExts: customSourceExts, | |
| extraNodeModules: { | |
| transformer: { | |
| babelTransformerPath: require.resolve('react-native-svg-transformer'), | |
| getTransformOptions: async () => ({ | |
| transform: { | |
| experimentalImportSupport: false, | |
| inlineRequires: true, | |
| }, | |
| }), | |
| }, | |
| resolver: { | |
| assetExts: newAssetExts, | |
| sourceExts: customSourceExts, | |
| extraNodeModules: { |
🤖 Prompt for AI Agents
In example/metro.config.js around lines 13 to 24, SVGs were moved from assetExts
to sourceExts but no transformer was configured; add a babelTransformerPath that
points to an SVG JS transformer (e.g. react-native-svg-transformer) and ensure
transformer config routes .svg files through that transformer so imports like
import './icon.svg' are parsed; keep .svg removed from assetExts, added to
sourceExts, and set transformer.babelTransformerPath to the installed svg
transformer module.
| if [[ -d "$PROJECT_ROOT/example/android" ]]; then | ||
| cd "$PROJECT_ROOT/example/android" | ||
| ./gradlew --stop 2>/dev/null || true | ||
| fi | ||
| if [[ -d "$PROJECT_ROOT/sample/android" ]]; then | ||
| cd "$PROJECT_ROOT/sample/android" | ||
| ./gradlew --stop 2>/dev/null || true | ||
| fi |
There was a problem hiding this comment.
Add error handling for directory changes.
The cd commands when stopping Gradle daemons should have error handling.
Apply this diff:
if [[ -d "$PROJECT_ROOT/example/android" ]]; then
- cd "$PROJECT_ROOT/example/android"
- ./gradlew --stop 2>/dev/null || true
+ (cd "$PROJECT_ROOT/example/android" && ./gradlew --stop 2>/dev/null) || true
fi
if [[ -d "$PROJECT_ROOT/sample/android" ]]; then
- cd "$PROJECT_ROOT/sample/android"
- ./gradlew --stop 2>/dev/null || true
+ (cd "$PROJECT_ROOT/sample/android" && ./gradlew --stop 2>/dev/null) || true
fiNote: Using subshells eliminates the need to track and restore the working directory.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ -d "$PROJECT_ROOT/example/android" ]]; then | |
| cd "$PROJECT_ROOT/example/android" | |
| ./gradlew --stop 2>/dev/null || true | |
| fi | |
| if [[ -d "$PROJECT_ROOT/sample/android" ]]; then | |
| cd "$PROJECT_ROOT/sample/android" | |
| ./gradlew --stop 2>/dev/null || true | |
| fi | |
| if [[ -d "$PROJECT_ROOT/example/android" ]]; then | |
| (cd "$PROJECT_ROOT/example/android" && ./gradlew --stop 2>/dev/null) || true | |
| fi | |
| if [[ -d "$PROJECT_ROOT/sample/android" ]]; then | |
| (cd "$PROJECT_ROOT/sample/android" && ./gradlew --stop 2>/dev/null) || true | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 95-95: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 99-99: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In scripts/clean-all-caches.sh around lines 94-101, replace the bare cd calls
with subshell invocations that handle cd failures; e.g. wrap each operation as
(cd "$PROJECT_ROOT/example/android" && ./gradlew --stop 2>/dev/null) || { echo
"Failed to enter $PROJECT_ROOT/example/android" >&2; true; } and similarly for
the sample/android path so the working directory isn't changed and any cd
failure is reported and safely ignored.
| fi | ||
|
|
||
| # Return to original directory | ||
| cd "$CURRENT_DIR" |
There was a problem hiding this comment.
Add error handling for directory change.
While this is cleanup, consistent error handling is recommended.
Apply this diff:
-cd "$CURRENT_DIR"
+cd "$CURRENT_DIR" || true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd "$CURRENT_DIR" | |
| cd "$CURRENT_DIR" || true |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 104-104: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In scripts/clean-all-caches.sh around line 104, the script does cd
"$CURRENT_DIR" without checking for failure; add error handling by verifying the
cd succeeded (e.g., test the exit status or use ||) and if it fails emit a clear
error message to stderr and exit with a non-zero status so the cleanup script
does not continue in the wrong directory.
|
|
||
| # Install dependencies in root folder | ||
| echo "🏠 Installing dependencies in root folder..." | ||
| cd "$PROJECT_ROOT" |
There was a problem hiding this comment.
Add error handling for directory changes.
The cd command can fail if the directory doesn't exist or isn't accessible. Without error handling, subsequent commands would execute in the wrong directory, potentially causing unexpected behavior.
Apply this diff to add error handling:
-cd "$PROJECT_ROOT"
+cd "$PROJECT_ROOT" || exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd "$PROJECT_ROOT" | |
| cd "$PROJECT_ROOT" || exit 1 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 32-32: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In scripts/install-dependencies.sh around line 32, the cd "$PROJECT_ROOT" call
lacks error handling; update the script so that if the cd fails it prints a
clear error message to stderr and terminates the script with a non‑zero exit
code (for example, check the exit status of cd and exit immediately on failure).
Ensure the error message includes the target directory path for debugging and
that subsequent commands do not run when changing directory fails.
|
|
||
| # Install dependencies in example folder | ||
| echo "📱 Installing dependencies in example folder..." | ||
| cd "$PROJECT_ROOT/example" |
There was a problem hiding this comment.
Add error handling for directory changes.
The cd command should have error handling to prevent executing subsequent commands in the wrong directory if the change fails.
Apply this diff to add error handling:
-cd "$PROJECT_ROOT/example"
+cd "$PROJECT_ROOT/example" || exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd "$PROJECT_ROOT/example" | |
| cd "$PROJECT_ROOT/example" || exit 1 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 42-42: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In scripts/install-dependencies.sh around line 42, the single-line cd
"$PROJECT_ROOT/example" lacks error handling; change it to check cd's exit
status and abort if it fails (for example, replace the bare cd with a guarded
command that echoes a clear error message and exits with a non-zero status when
cd fails, e.g. cd "$PROJECT_ROOT/example" || { echo "Failed to change directory
to $PROJECT_ROOT/example"; exit 1; }).
| fi | ||
|
|
||
| # Return to original directory | ||
| cd "$CURRENT_DIR" |
There was a problem hiding this comment.
Add error handling for directory changes.
While this is a cleanup step, the cd command should still have error handling for consistency and to prevent potential issues.
Apply this diff to add error handling:
-cd "$CURRENT_DIR"
+cd "$CURRENT_DIR" || return 0Note: Using return 0 here since this is cleanup and we don't want to fail the script if we can't return to the original directory.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd "$CURRENT_DIR" | |
| cd "$CURRENT_DIR" || return 0 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 51-51: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In scripts/install-dependencies.sh around line 51, the plain cd "$CURRENT_DIR"
lacks error handling; update it to attempt to change back into $CURRENT_DIR and,
if that fails, print a brief error message to stderr (e.g., echo to >&2) and
then return 0 so cleanup does not cause the script to fail.
Summary by CodeRabbit
New Features
Documentation
Chores