Skip to content
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

App Check no longer compiles as an Objective-C++ module as of 10.16.0 #11916

Closed
mikehardy opened this issue Oct 10, 2023 · 6 comments · Fixed by #11860
Closed

App Check no longer compiles as an Objective-C++ module as of 10.16.0 #11916

mikehardy opened this issue Oct 10, 2023 · 6 comments · Fixed by #11860
Assignees

Comments

@mikehardy
Copy link
Contributor

Per Paul B - this has already been noticed internally and is in progress, he mentioned I could open an issue for tracking and I'd like to do so

My original comment on the PR between firebase-ios-sdk 10.15.0 and 10.16.0 follows:


When I attempt to update react-native-firebase to firebase-ios-sdk 10.16.0, including this PR, I receive this:

❌ /Users/mike/work/invertase/react-native-firebase/tests/ios/build/Build/Products/Debug-iphonesimulator/FirebaseAppCheck/FirebaseAppCheck.framework/Headers/FIRAppCheck.h:19:1: use of '@import' when C++ modules are disabled, consider using -fmodules and -fcxx-modules
@import FirebaseAppCheckInterop;
^

I traced it to this PR when I noticed that header file changed in my pod cache between 10.15.0 and 10.16.0. Is this an expected outcome? (stated differently, is there something I need to change so this works, ideally that wouldn't be a breaking change for us since we're adopting a feature release here...)

It's coming from here:
https://github.com/invertase/react-native-firebase/blob/c9b695aa8fd75d5a1d070ecbb6bb9ac4e9ff062e/tests/ios/testing/AppDelegate.mm#L21
Which goes to here:
https://github.com/invertase/react-native-firebase/blob/c9b695aa8fd75d5a1d070ecbb6bb9ac4e9ff062e/packages/app-check/ios/RNFBAppCheck/RNFBAppCheckModule.h#L22
Which goes to here:
https://github.com/invertase/react-native-firebase/blob/c9b695aa8fd75d5a1d070ecbb6bb9ac4e9ff062e/packages/app-check/ios/RNFBAppCheck/RNFBAppCheckProviderFactory.h#L18

...and that file doesn't go through with our compiler settings 🤔

Is there anything that can be done here?

In issues past, I have attempted to provide build instructions for our test app over in react-native-firebase so reproductions are easy but I/we have observed you all may have difficulty since it requires a relatively up to date javascript / node environment etc etc and that's a time suck if you don't already have it running. Let me know if you want repro instructions or any other information though, happy to collaborate of course

Originally posted by @mikehardy in #11660 (comment)

@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@paulb777 paulb777 changed the title App Check no longer compiles as an Objective-C module as of 10.16.0 App Check no longer compiles as an Objective-C++ module as of 10.16.0 Oct 10, 2023
@paulb777 paulb777 added this to the 10.17.0 - M139 milestone Oct 10, 2023
@mikehardy
Copy link
Contributor Author

mikehardy commented Oct 10, 2023

Just as a release note - this puts us in a little bit of a bind release-wise with a crash bug in Firestore from the gRPC library change making 10.16.0 a desirable update contrasted with an inability to recommend it as a general thing with an app check compile failure.

It does not appear possible to use podspec magic in our react-native-firebase podspec to alter the gRPC transitive dependency version from FirebaseFirestore.podspec, nor does it appear possible to override the gRPC pod version to 1.49.1 in an app's Podfile. If I am wrong in that I'd welcome the correction so we could provide guidance to fix firestore crashes

If I am right that there is no easy way to override the gRPC update in 10.15.0 firebase-ios-sdk release then it would be nice to see either a 10.15.1 that included just the gRPC change as a cherry-pick or to see a 10.16.1 (or 10.17.0 I suppose) that just had the compile fix so that either could come sooner. This may have all already been taken into account, but I mention it just in case. Cheers

@paulb777
Copy link
Member

I'm not recommending this, but the right podspec magic might be to copy the FirebaseFirestore.podspec, change the grpc dependency, rename the podspec, publish it and depend on that pod from react-native.

@paulb777
Copy link
Member

@mikehardy Is there a reason that react-native needs to integrate via Objective-C++ versus Objective-C? If not, using Objective C could be a workaround.

@mikehardy
Copy link
Contributor Author

React-native is C++ now, they use code generators etc, all of which results in lots of C++ in the system so that's outside our ability to change I believe

@paulb777
Copy link
Member

FYI, we plan to have a fix in the 10.17.0 release due out the week of October 23rd. We considered a patch, but determined the change is too far-ranging and should go through the full minor release cycle.

@firebase firebase locked and limited conversation to collaborators Nov 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants