-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix(app-check): use bridging header instead of Swift module import #8867
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
fix(app-check): use bridging header instead of Swift module import #8867
Conversation
The `RNFBAppCheck` pod is an Obj-C static library without `DEFINES_MODULE` or a module map, so `import RNFBAppCheck` in Swift AppDelegate fails with "No such module 'RNFBAppCheck'" when using `useFrameworks: "static"` (required by firebase-ios-sdk). This changes the Expo config plugin to: 1. Remove `import RNFBAppCheck` from AppDelegate.swift 2. Add `#import <RNFBAppCheckModule.h>` to the bridging header The Obj-C class `RNFBAppCheckModule` is then available to Swift through the bridging header, which is how Obj-C pods without module maps should be consumed from Swift. Fixes invertase#8700 Fixes invertase#8757 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@safaiyeh is attempting to deploy a commit to the Invertase Team on Vercel. A member of the Team first needs to authorize it. |
mikehardy
left a comment
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.
@safaiyeh you are the hero we needed, thanks so much for this. App-Check + Expo 54 was the last item on my todo list to feel like we supported Expo well at a technical level (with https://github.com/mikehardy/rnfbdemo/blob/main/make-expo-demo.sh to prove it). We still have a ways to go on the documentation side but simply having it all working (and demonstrated with a build demo) is a huge step for us and this was all that's missing.
Worked great when I tested it locally
|
it only affects the plugin so none of the e2e test results will be important and they are flaky right now (see #8865 ) - that won't gate merge. Just need lint and jest to pass - I see jest failed locally for me because it's verifying an import that isn't there anymore, I'll fix that up and get this in since I already qualified that it worked locally |
… tests the existing tests needed a little fix after the Swift version of the app-check mod was converted to altering the bridging header, and I added a new test that was bridging header specific
|
Glad I could help! |
Summary
The
RNFBAppCheckpod is an Obj-C static library that doesn't setDEFINES_MODULEor include a module map. When projects useuseFrameworks: "static"(required byfirebase-ios-sdk), the Expo config plugin'simport RNFBAppCheckinAppDelegate.swiftfails with:This is a widely reported issue:
Changes
Updates the Expo config plugin (
plugin/src/ios/appDelegate.ts) to:import RNFBAppCheckfromAppDelegate.swift(including cleaning up any previously added imports)#import <RNFBAppCheckModule.h>to the project's bridging header (ProjectName-Bridging-Header.h)The
RNFBAppCheckModuleclass is then available to Swift through the bridging header, which is the standard way to consume Obj-C pods without module maps from Swift.Why not add
DEFINES_MODULE?Adding
DEFINES_MODULE = YESto the podspec would also fix the Swift import, but would be a larger change affecting all consumers. The bridging header approach is the minimal, targeted fix that matches how the Obj-C path already works (using#import).Testing
expo prebuild --platform ios --cleanthatimport RNFBAppCheckis removed and bridging header is updatedxcodebuild buildsucceeds@react-native-firebase/app-check23.8.4Fixes #8700
Fixes #8757