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

fix(app, ios): avoid photo API not present on Catalyst #4328

Merged
merged 1 commit into from
Sep 30, 2020
Merged

fix(app, ios): avoid photo API not present on Catalyst #4328

merged 1 commit into from
Sep 30, 2020

Conversation

kyle-ssg
Copy link
Contributor

@kyle-ssg kyle-ssg commented Sep 30, 2020

Description

As it stands, firebase app causes compilation errors in Mac OS, I believe this is the way to fix it. There does seem to be an attempt to solve it I think currently but standard conditionals would not get evaluated at compile time (Cue someone with more than 5 hours worth of Objective C experience.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.

    • Yes
  • My change supports the following platforms;

    • Android
    • iOS
    • Mac
  • My change includes tests;

    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.

  • This is a breaking change;

    • Yes
    • No

Test Plan


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@CLAassistant
Copy link

CLAassistant commented Sep 30, 2020

CLA assistant check
All committers have signed the CLA.

@vercel
Copy link

vercel bot commented Sep 30, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/bgpzmdraw
✅ Preview: https://react-native-firebase-git-fork-bullettrainhq-master.invertase.vercel.app

@kyle-ssg kyle-ssg changed the title Fix build for Mac Catalyst fix(app) build for Mac Catalyst Sep 30, 2020
@kyle-ssg kyle-ssg changed the title fix(app) build for Mac Catalyst fix(app): build for Mac Catalyst Sep 30, 2020
@mikehardy
Copy link
Collaborator

This seems reasonable to me - can you sign the CLA @kyle-ssg ? I'll look into the E2E errors in CI they are almost certainly unrelated

@mikehardy
Copy link
Collaborator

Indeed, CI failure totally unrelated. And CI passed on iOS which is the location of this change. For posterity:

09-30 08:38:34.863 12160 12160 D AndroidRuntime: Shutting down VM
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: Exception in native call
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: java.lang.IllegalStateException: FirebaseFirestore has already been started and its settings can no longer be changed. You can only call setFirestoreSettings() before calling any other methods on a FirebaseFirestore object.
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at com.google.firebase.firestore.FirebaseFirestore.setFirestoreSettings(FirebaseFirestore.java:191)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at io.invertase.firebase.firestore.UniversalFirebaseFirestoreCommon.setFirestoreSettings(UniversalFirebaseFirestoreCommon.java:93)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at io.invertase.firebase.firestore.UniversalFirebaseFirestoreCommon.getFirestoreForApp(UniversalFirebaseFirestoreCommon.java:45)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at io.invertase.firebase.firestore.UniversalFirebaseFirestoreModule.clearPersistence(UniversalFirebaseFirestoreModule.java:83)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at io.invertase.firebase.firestore.ReactNativeFirebaseFirestoreModule.clearPersistence(ReactNativeFirebaseFirestoreModule.java:50)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at java.lang.reflect.Method.invoke(Native Method)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:151)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at android.os.Handler.handleCallback(Handler.java:873)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at android.os.Handler.dispatchMessage(Handler.java:99)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at android.os.Looper.loop(Looper.java:193)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:226)
09-30 08:38:35.327  3837  9126 E unknown:ReactNative: 	at java.lang.Thread.run(Thread.java:764)
09-30 08:38:35.381 12170 12170 D AndroidRuntime: >>>>>> START com.android.internal.os.RuntimeInit uid 2000 <<<<<<
09-30 08:38:35.439  3837  3837 D ReactNative: CatalystInstanceImpl.destroy() start
09-30 08:38:35.439  3837  3837 D ReactNative: CatalystInstanceImpl.destroyV1() start
09-30 08:38:35.449  3837  9126 D Auth    : instance-destroyed

Related already tracking here: #4058 (comment)

@mikehardy mikehardy changed the title fix(app): build for Mac Catalyst fix(app, ios): avoid photo API not present on Catalyst Sep 30, 2020
@mikehardy mikehardy merged commit 86f1f63 into invertase:master Sep 30, 2020
@mikehardy
Copy link
Collaborator

Thanks for this @kyle-ssg - working on a release as the next step but need to complete my root cause analysis on what caused the previous publish to fail #4283

@mikehardy
Copy link
Collaborator

published

androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants