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(ios): exclude configure if FirApp is already configured #321

Merged
merged 1 commit into from
Dec 7, 2024

Conversation

mrdj07
Copy link

@mrdj07 mrdj07 commented Dec 3, 2024

Exclude configure if FirApp is already configured by an other library/

Description

Using your package, especially your latest release 5.0.0, I encountered an error with Firebase being already configured. I used the FirApp verification from an other Cordova Plugin, tested it in my project and it works great.

@erisu
Copy link

erisu commented Dec 4, 2024

I’m wondering if we should use reflection to call isDefaultAppConfigured from FIRApp instead of using [FIRApp defaultApp]. Though this might not be recommended.

Using [FIRApp defaultApp] will solve the issue you're seeing and works well if there are multiple plugins that call [FIRApp configure].

However, if this is the only plugin using Firebase, or if this plugin loads before others, an error log message is displayed due to calling [FIRApp defaultApp] before the app is configured. This message might be seen as an error, even though it's just noise. Some users have reported this issue in the past.

Here’s an example of what would be printed in the log:

10.24.0 - [FirebaseCore][I-COR000003] The default Firebase app has not yet been configured. Add `FirebaseApp.configure()` to your application initialization. This can be done in in the App Delegate's application(_:didFinishLaunchingWithOptions:)` (or the `@main` struct's initializer in SwiftUI). Read more: https://goo.gl/ctyzm8.

This message comes from the defaultApp method: https://github.com/firebase/firebase-ios-sdk/blob/main/FirebaseCore/Sources/FIRApp.m#L226-L239

isDefaultAppConfigured seems to simply check whether the app is configured, returning a boolean without the warning. However, the method has been converted to private, and the Firebase team has decided not to expose it, as they "always want Firebase configuration to be explicit."

@erisu erisu linked an issue Dec 5, 2024 that may be closed by this pull request
@erisu erisu added bug Something isn't working ios Related to the iOS platform labels Dec 5, 2024
@erisu erisu added this to the 5.0.1 milestone Dec 5, 2024
@mrdj07
Copy link
Author

mrdj07 commented Dec 6, 2024

Okay so if isDefaultAppConfigured is private, and using [FIRApp defaultApp] seems not ideal to you, how do you think we can solve the double Firebase configuration issue ?

Copy link

@erisu erisu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will merge this PR in as is. No changes needed.

Later I will open a ticket on the Firebase repo to see if they would reconsider making that other method public or if they have alternative solutions that does not display the error message.

@mrdj07
Copy link
Author

mrdj07 commented Dec 6, 2024

Fantastic, thank you for the detailed explanation and follow up ! I am not an iOS developper at all and am struggling to keep an cordova app running haha !

@erisu erisu merged commit dbceb88 into havesource:master Dec 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ios Related to the iOS platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should not assume Firebase is not already configured.
2 participants