Chore/msdk 3145 fix ios react native new arch #173
Conversation
|
CodeAnt AI is reviewing your PR. |
WalkthroughBuild system configuration updates across Android and iOS platforms to support React Native new architecture. Changes include CMake target renaming, dynamic C++ build flags for iOS, updated header search paths, framework linking, and plist configuration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
codeant-ai
bot
added
the
size:M
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
|
PR Summary: Short summary: Key changes:
Potential impacts / notes:
|
Nitpicks 🔍
|
|
Reviewed up to commit:ff27bce200c7b5d7958a1ef697167aa042888364 Looks good to me! 👍 Additional SuggestionOthers- You renamed the library target from the previous react_codegen_RNUsercentricsModule to rn_usercentrics. Codegen and RN's new-architecture registration (symbols and expected library names) may rely on the previous naming conventions. Confirm nothing else (other build scripts, Java/Kotlin registration, or consumers) expects the old target/name. If codegen or the RN CLI expects the previous name, revert the change or update all consumers accordingly (see original target name in reference lines 1-20). - You changed the linked/native dependency helpers in a way that will likely break CMake configuration or link resolution on many RN setups: (1) You call ReactAndroid::fbjni in the new branch but never find_package(fbjni) (original code linked fbjni::fbjni explicitly). Either call find_package(fbjni REQUIRED CONFIG) and link fbjni::fbjni (or ensure the ReactAndroid package actually exports ReactAndroid::fbjni on all targeted RN versions). (2) You replaced the original explicit list of RN native components used for older RN versions with a short list (ReactAndroid::reactnative / ReactAndroid::jsi). That may hide required symbols when building against RN versions that do not provide those aggregated targets. Revert to linking the explicit components for older RN versions or add feature-detection logic to choose the correct exported targets. See original CMake behavior in reference CMakeLists.txt (reference lines 21-85).# Find ReactAndroid package first (required for version detection)
find_package(ReactAndroid REQUIRED CONFIG)
# Also find fbjni if not provided by ReactAndroid
if(NOT TARGET ReactAndroid::fbjni)
find_package(fbjni REQUIRED CONFIG)
endif()
# Link libraries based on React Native version
if(DEFINED ReactAndroid_VERSION_MINOR AND ReactAndroid_VERSION_MINOR GREATER_EQUAL 76)
target_link_libraries(
${LIB_TARGET_NAME}
ReactAndroid::reactnative
ReactAndroid::jsi
ReactAndroid::fbjni
android
log
)
else()
# Fallback for older RN versions or when version is not available
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
android
log
)
endif()
# Compile options based on React Native version
if(COMMAND target_compile_reactnative_options)
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()
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
android/src/main/jni/CMakeLists.txt (1)
9-17: Duplicate compile options between global and target-specific scopes.
-fexceptions,-frtti,-std=c++20, and-Wallare defined both globally viaadd_compile_options(lines 9-17) and again viatarget_compile_options(lines 60-69). This redundancy doesn't break the build but adds unnecessary noise.Consider consolidating all compile options in one place. Since this is a single-target CMake file, you can remove the global
add_compile_optionsblock and keep onlytarget_compile_options:-# 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)Then update the target-specific options to include the missing flags:
target_compile_options( ${LIB_TARGET_NAME} PRIVATE -DLOG_TAG=\"ReactNative\" -fexceptions -frtti -std=c++20 -Wall + -Wpedantic + -Wno-gnu-zero-variadic-macro-arguments + -Wno-dollar-in-identifier-extension )Also applies to: 60-69
react-native-usercentrics.podspec (1)
42-59: Consider adding trailing commas to multiline arrays.Per Ruby style conventions, trailing commas in multiline arrays improve diff readability when items are added.
header_search_paths = [ '$(PODS_ROOT)/React-Core', '$(PODS_ROOT)/React-NativeModulesApple', '$(PODS_ROOT)/ReactCommon', '$(PODS_CONFIGURATION_BUILD_DIR)/ReactCommon', '$(PODS_TARGET_SRCROOT)', '$(PODS_ROOT)/Headers/Public', '$(PODS_ROOT)/Headers/Public/React-Core', - '$(PODS_ROOT)/Headers/Public/ReactCommon' + '$(PODS_ROOT)/Headers/Public/ReactCommon', ] # Add new architecture paths if available (will be resolved at build time) header_search_paths += [ '$(PODS_ROOT)/Headers/Public/React-Fabric', '$(PODS_ROOT)/Headers/Public/React-Codegen', '$(PODS_CONFIGURATION_BUILD_DIR)/React-Codegen/React_Codegen.framework/Headers', - '$(PODS_CONFIGURATION_BUILD_DIR)/React-Fabric/React_Fabric.framework/Headers' + '$(PODS_CONFIGURATION_BUILD_DIR)/React-Fabric/React_Fabric.framework/Headers', ]sample/ios/sample.xcodeproj/project.pbxproj (1)
523-526: Unusual empty space in OTHER_LDFLAGS.The
OTHER_LDFLAGSarray contains" "(a single space) which appears unintentional. This won't break the build but is unnecessary.OTHER_LDFLAGS = ( "$(inherited)", - " ", );The same applies to the Release configuration at lines 612-615.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
android/src/main/jni/CMakeLists.txt(2 hunks)react-native-usercentrics.podspec(1 hunks)sample/ios/sample.xcodeproj/project.pbxproj(9 hunks)sample/ios/sample/Info.plist(1 hunks)
🧰 Additional context used
🪛 RuboCop (1.81.7)
react-native-usercentrics.podspec
[convention] 50-50: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 58-58: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-android
- GitHub Check: test-ios
🔇 Additional comments (4)
sample/ios/sample/Info.plist (1)
36-37: LGTM - New architecture flag correctly added.Adding
RCTNewArchEnabledset tofalsein the sample app's Info.plist is a valid approach for controlling the React Native new architecture feature at the app level.android/src/main/jni/CMakeLists.txt (1)
36-58: Version-based linking approach looks correct.The conditional linking based on
ReactAndroid_VERSION_MINORproperly handles both React Native 0.76+ (withfbjni) and older versions. TheDEFINEDcheck guards against missing version info.react-native-usercentrics.podspec (1)
24-67: Good approach for Expo/React Native new architecture compatibility.Using
$(inherited)forGCC_PREPROCESSOR_DEFINITIONSandOTHER_CPLUSPLUSFLAGSallows the parent project (Expo or React Native CLI) to control theRCT_NEW_ARCH_ENABLEDflag, which is the correct pattern for library podspecs.sample/ios/sample.xcodeproj/project.pbxproj (1)
498-500: React-runtimeexecutor header paths correctly added.Adding the
React-runtimeexecutorframework headers to the search paths is appropriate for React Native new architecture support.
There was a problem hiding this comment.
You can revert this file
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
You can revert this file
|
CodeAnt AI is running Incremental review |
codeant-ai
bot
added
size:M
|
CodeAnt AI Incremental review completed. |
User description
User description
fix react native for iOS new arch bug
PR Type
Bug fix, Enhancement
Description
Fix iOS React Native new architecture compatibility by inheriting RCT_NEW_ARCH_ENABLED from parent project instead of hardcoding to 0
Refactor podspec C++ configuration with improved header search paths and modular build settings
Simplify Android CMakeLists.txt to use ReactAndroid package with version detection and fallback support
Update Xcode project configuration with React-runtimeexecutor headers and framework references
Diagram Walkthrough
File Walkthrough
react-native-usercentrics.podspec
Refactor podspec for new architecture compatibilityreact-native-usercentrics.podspec
base_cpp_flagsdictionarywith improved organization
GCC_PREPROCESSOR_DEFINITIONSfrom hardcodedRCT_NEW_ARCH_ENABLED=0to$(inherited)to allow parent project controland standard library paths for new architecture support
CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES,DEFINES_MODULE,USE_HEADERMAP,ALWAYS_SEARCH_USER_PATHSOTHER_CPLUSPLUSFLAGSto include$(inherited)for both pod anduser target configurations
CMakeLists.txt
Simplify CMake configuration with ReactAndroid packageandroid/src/main/jni/CMakeLists.txt
project(rn_usercentrics)declaration and changed library namefrom
react_codegen_RNUsercentricsModuletorn_usercentricsfind_package(ReactAndroid REQUIRED CONFIG)to properly detectReact Native version
ReactAndroid::namespaced targetswith version-based conditional logic
standard approach using
-std=c++20,-fexceptions,-frtti${CMAKE_PROJECT_NAME}to${LIB_TARGET_NAME}in include directoriesproject.pbxproj
Update Xcode project with new framework referencessample/ios/sample.xcodeproj/project.pbxproj
918B0A97B42EE4DB5846EED8forPods_sample.frameworkandD8194E5BF509496886EB6B66forPods_sampleTests.framework82EB8D8E4ED3004B625C802Bsample-Bridging-Header.hfrom project group structure[CP] Check Pods Manifest.lockshell script build phaseReact-runtimeexecutorframework headers toHEADER_SEARCH_PATHSin both Debug and Release configurations
OTHER_LDFLAGSfrom string to array format for consistencyInfo.plist
Disable new architecture in sample appsample/ios/sample/Info.plist
RCTNewArchEnabledkey with boolean valuefalseto explicitlydisable new architecture in sample app
CodeAnt-AI Description
Restore React Native iOS new architecture support and stabilize Android builds
What Changed
Impact
✅ Successful iOS builds with React Native new architecture✅ Fewer Android build and linkage errors✅ Smoother React Native upgrades for the SDK integration💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.