-
Notifications
You must be signed in to change notification settings - Fork 4.1k
fix: a bug with the demoProjectId
arg to Firebase.initializeApp()
#17703
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
Conversation
ce1b505
to
bf99017
Compare
demoProjectId
arg to Firebase.initializeApp()
demoProjectId
arg to Firebase.initializeApp()
c5d7148
to
f550d96
Compare
Ok, this is ready for review now. |
f550d96
to
2cd1e6b
Compare
I made one minor tweak to the docs. It's ready for real this time. |
If platform specific configuration files exist for the project (i.e. GoogleService-Info.plist for iOS), the default `MethodChannelFirebaseApp` is initialized using the configuration it specifies. This is fine since multiple `FirebaseApp` instances can exist, however when `MethodChannelFirebase.initializeApp()` is called it uses the default app name (`[DEFAULT]`, from the code) if no name is passed as an argument. Since the default app is initialized using the platform specific configuration before `initializeApp()` is called, the user provided options are ignored. The solution is to ensure a non-null app name is passed to `initializeApp()` to avoid conflicts with platform specific configuration files when the user intends to use a "demo-" project for testing. Since we should be providing a distinct app name for the demo app, the allowance for mismatched options between the specified options and the existing options for demo apps is also removed.
90feb9f
to
e8fbff7
Compare
I'm not sure how this change could cause the e2e test failures, but I've synced to the main branch. Please allow them to rerun. |
Looks like all of the checks passed except for the web one, which was cancelled. Let me know if there's anything I need to do. Thanks |
On Android the underlying Java SDK will throw an exception if the api key is empty. Setting it to any arbitrary value is sufficient.
e7e0b77
to
54e08d1
Compare
Sorry for the late change, but I pushed one more commit. I found that the iOS SDK will tolerate an empty api key, but the Android SDK throws an exception if the api key is set to an empty string. Setting any arbitrary value is sufficient for it to work, which is what I'm doing in the new commit. |
The e2e test failure seems unrelated:
|
Hi - what do we need to do to get this submitted? Please let me know. |
If platform specific configuration files exist for the project (i.e. GoogleService-Info.plist for iOS), the default
MethodChannelFirebaseApp
is initialized using the configuration it specifies. This is fine since multipleFirebaseApp
instances can exist, however whenMethodChannelFirebase.initializeApp()
is called it uses the default app name ([DEFAULT]
, from the code) if no name is passed as an argument. Since the default app is initialized using the platform specific configuration beforeinitializeApp()
is called, the user provided options are ignored.The solution is to ensure a non-null app name is passed to
initializeApp()
to avoid conflicts with platform specific configuration files when the user intends to use a "demo-" project for testing.Since we should be providing a distinct app name for the demo app, the allowance for mismatched options between the specified options and the existing options for demo apps is also removed.
Description
Replace this paragraph with a description of what this PR is doing. If you're modifying existing behavior, describe the existing behavior, how this PR is changing it, and what motivated the change.
Related Issues
Replace this paragraph with a list of issues related to this PR from the issue database. Indicate, which of these issues are resolved or fixed by this PR. Note that you'll have to prefix the issue numbers with flutter/flutter#.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
).This will ensure a smooth and quick review process. Updating the
pubspec.yaml
and changelogs is not required.///
).melos run analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?