fix(ios): resolve duplicate GUL/Firebase classes under Expo SPM dynamic frameworks - #9326
Draft
russellwheatley wants to merge 1 commit into
Draft
russellwheatley wants to merge 1 commit into
russellwheatley wants to merge 1 commit into
Conversation
…ic frameworks Demonstrates a fix for duplicate GUL*/Firebase classes compiled into both RNFBAnalytics.framework and RNFBMessaging.framework under Expo's SPM + useFrameworks: dynamic path (#9322), using the local dynamic umbrella a firebase-ios-sdk maintainer suggested (since firebase-ios-sdk itself can't ship one, firebase/firebase-ios-sdk#8945). Once Firebase goes through the umbrella, the individual Firebase headers are hidden from Analytics's and Messaging's .mm/.m files, so their __has_include guards fall back to @import FirebaseAnalytics/@import FirebaseMessaging, illegal in Objective-C++ without -fcxx-modules (which in turn breaks React Native's own build). To get a clean build under the umbrella: - Move every direct Firebase Analytics/Messaging call out of .mm/.m and behind a small Swift facade (RNFBAnalyticsFacade, RNFBMessagingFacade), the same pattern RNFBFunctionsCallHandler already used. - Rewrite the FCM token-refresh delegate in Swift, split into an internal implementation (conforms to Firebase's MessagingDelegate) and a thin public wrapper, since a public Swift type conforming to a hidden framework's protocol forces Swift to add its own illegal @import to the generated header. - Add a same-pod Objective-C bridge (RNFBMessagingEventEmitter) so the Swift delegate can still reach RNFBApp's RNFBRCTEventEmitter, since Swift can't import a sibling RNFB pod directly (every RNFB pod marks nearly all its headers private). - Give RNFBAnalytics an explicit framework search path and -framework GoogleAppMeasurement: FirebaseAnalytics resolves to a prebuilt XCFramework rather than plain source, so it doesn't link automatically under the umbrella. yarn test-expo:ios:link now builds clean, and a new assert_dynamic_umbrella_graph check confirms none of RNFBApp/RNFBAnalytics/RNFBMessaging define their own FIRApp/GUL* symbols, and all three link the shared dynamic RNFBFirebase umbrella rather than embedding their own copy. This is a draft for discussion, not ready to merge: RNFBApp has its own separate fix and isn't reworked here, and this only covers the Expo/SPM + dynamic frameworks path, not CocoaPods-only installs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Demonstrates a fix for duplicate
GUL*/Firebase classes compiled into bothRNFBAnalytics.frameworkandRNFBMessaging.frameworkunder Expo's SPM +useFrameworks: dynamicpath (#9322), using the local dynamic umbrella a firebase-ios-sdk maintainer suggested (since firebase-ios-sdk itself can't ship one, firebase/firebase-ios-sdk#8945).Once Firebase goes through the umbrella, the individual Firebase headers are hidden from Analytics's and Messaging's
.mm/.mfiles, so their__has_includeguards fall back to@import FirebaseAnalytics/@import FirebaseMessaging, illegal in Objective-C++ without-fcxx-modules(which in turn breaks React Native's own build).To get a clean build under the umbrella:
.mm/.mand behind a small Swift facade (RNFBAnalyticsFacade,RNFBMessagingFacade), the same patternRNFBFunctionsCallHandleralready used.MessagingDelegate) and a thin public wrapper, since a public Swift type conforming to a hidden framework's protocol forces Swift to add its own illegal@importto the generated header.RNFBMessagingEventEmitter) so the Swift delegate can still reachRNFBApp'sRNFBRCTEventEmitter, since Swift can'timporta sibling RNFB pod directly (every RNFB pod marks nearly all its headers private).RNFBAnalyticsan explicit framework search path and-framework GoogleAppMeasurement:FirebaseAnalyticsresolves to a prebuilt XCFramework rather than plain source, so it doesn't link automatically under the umbrella.yarn test-expo:ios:linknow builds clean, and a newassert_dynamic_umbrella_graphcheck confirms none ofRNFBApp/RNFBAnalytics/RNFBMessagingdefine their ownFIRApp/GUL*symbols, and all three link the shared dynamicRNFBFirebaseumbrella rather than embedding their own copy.This is a draft for discussion, not ready to merge:
RNFBApphas its own separate fix and isn't reworked here, and this only covers the Expo/SPM + dynamic frameworks path.