invertase / firestore-ios-sdk-frameworks

⚡ Precompiled Firestore iOS SDKs extracted from the Firebase iOS SDK repository release downloads for faster build times.
Apache License 2.0
447 stars 49 forks source link

[SOLVED] Xcode 12.1 required w/Detox, inside Xcode - 12.0 does not work #15

Closed rborn closed 4 years ago

rborn commented 4 years ago

Hi

I'm trying to use this with detox but I get a lot of errors when building, starting with

/....../app/letsemjoyapp/node_modules/@react-native-firebase/firestore/ios/RNFBFirestore/RNFBFirestoreCommon.h:26:4: expected a type

+ (FIRFirestore *)getFirestoreForApp:(FIRApp *)firebaseApp;
^

My Detox build command is

xcodebuild -workspace ios/emjoy.xcworkspace -UseNewBuildSystem=NO -scheme emjoy -configuration Debug -sdk iphonesimulator -derivedDataPath ios/build | xcpretty

Any idea ? 😻

mikehardy commented 4 years ago

I actually have the same problem when I build from Xcode (12.0.1) but it works from the command line for me? Incidentally, 'UseNewBuildSystem=NO' why?

rborn commented 4 years ago

@mikehardy My project is still set with legacy, and it doesn't build with or without that flag. I'll check if I can remove it, but I don't think it will make any difference.

mikehardy commented 4 years ago

I agree it will likely affect nothing (unfortunately) All I can say for sure is that with this performance trick integrated on Xcode 12 I cannot successfully build via Xcode, but I can build just fine from the command line, I don't use detox though.

But! We use it in the E2E tests that run on CI for react-native-firebase: https://github.com/invertase/react-native-firebase/blob/f6e64ed5536a4ff3ac7680bc97c18ef8c824b72a/tests/ios/Podfile#L48

And they work there, with detox enabled, from the command line at least?

That is not definitive but it shows at least one integration where it is working That's with macOS latest: https://github.com/invertase/react-native-firebase/blob/f6e64ed5536a4ff3ac7680bc97c18ef8c824b72a/.github/workflows/tests_e2e.yml#L28

...which is still Xcode 11.7, so perhaps this is not representative of a great build setup at the moment though it is about to change: https://github.com/actions/virtual-environments/blob/main/images/macos/macos-10.15-Readme.md#xcode

I can't recommend going down from Xcode 12 but this may be the root of the problem - something different in the way they build

mikehardy commented 4 years ago

I just reproduced this while checking on something else, detox fails while this is integrated and you build with Xcode 12. It also won't build from Xcode even without detox, same errors.

It's fine with Xcode 11.

I welcome any suggestions...looks like it's a header path issue or something, but unfortunately I don't have the time + expertise to troubleshoot when the easy solution is to just comment that line in the Podfile (even though it means a longer build, of course)

mikehardy commented 4 years ago

In case I wasn't explicit enough: I don't have time to solve this now but would love a solution. My best guess is something changed with regard to header search path between Xcode 11 / Xcode 12 and something in the podspec needs a tweak

If I had time I'd open Xcode after attempting integration, reproduce the failure, then find the headers on disk via Terminal and trial and error where to put them, and it would be terrible and slow because I am terrible and slow at Objective-C and Xcode... thus the 🙏 for a PR

rborn commented 4 years ago

I'm in the same boat, just disabled it for now, will try to see if I can fix (my objC-fu is veeeery rusty 😅 )

mikehardy commented 4 years ago

on the plus side for reproduction I saw the exact same messages just compiling my work project with it integrated in Xcode 12, no detox needed. So it should be glaringly obvious and if I had any Obj-C fu it'd be fixed already haha

rborn commented 4 years ago

@mikehardy I've updated the rnfirebase/app to 8.4.6 , the precompiled sdk to 6.34.0 , Xcode to 12.1 and it seems to compile fine now with xcode (detox). I didn't make a release yet but I'll let you know when I do.

mikehardy commented 4 years ago

That's great news. I'm guessing 12.1 was the trick then as I already had the others I think. I have one machine still 12.0 and one 12.1 so I can confirm that later. Thanks for the report!

mikehardy commented 4 years ago

I confirm 12.1 works with this enabled and Detox. I no longer have problems using the framework, I'm going to happily close this. It was a 12.0 thing