Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[firebase_core] Add platform interface #1324
[firebase_core] Add platform interface #1324
Changes from 5 commits
480cb20
f1703bb
1a0a5a9
3b47add
2112a41
df8f5c4
4ee2edc
c8d5bd3
fc51b1f
2bb9878
def089b
c6df031
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This relates to #1392 which @kroikie just landed.
Generally in Flutterfire we try to match the native Firebase APIs as closely as we can while remaining idiomatic to the target platform. There's no equivalent to the FirebaseAppData class in the native Firebase API. There's just
FirebaseApp
and our Dart version of it exposes aFuture<FirebaseOptions> options
getter.The reason why we put an asynchronous
options
on firebase_core'sFirebaseApp
is that we wanted the developer to be able to sayFirebaseApp.instance
and start using thatapp
immediately without callingawait
.It seems odd, although not necessarily wrong to have a class
FirebaseOptions
that matches the naming in the app-facing API exactly and is designed to be exported, but then have another classFirebaseAppData
that serves a similar function toFirebaseApp
but has a slightly different class behavior and can't be exported. It seems that the main difference is thatFirebaseOptions
is a pure data class whereasFirebaseApp
has some behavior.It seems like for every class we have the following options. All of them avoid exposing nonstandard classes (
FirebaseAppData
) to the end developer.FirebaseOptions
but makes it harder to make breaking changes in firebase_core because they might also break firebase_core_platform_interface.FirebaseApp
but don't require it to match firebase_core'sFirebaseApp
. The same could apply toFirebaseOptions
. It could be confusing to have two identically named classes that are subtly different, but it makes it clear that both classes refer to the same native SDK concept on different layers. One of them could extend the other without polluting public API with classes that don't have a native SDK equivalent.FirebaseAppData
orPlatformFirebaseApp
). This might make make the code more readable within the app-facing package because it's obvious that the class is a more primitive representation of an app-facing class. However, it isn't possible to extend the platform class without exposing a platform implementation base class to the end developer.Right now you're doing Option 1 for
FirebaseOptions
and Option 3 forFirebaseApp
. Starting off with Option 1 and then moving to Option 2 or 3 later is possible, although here it would force the platform interface to use the same asynchronous API for options.Here's a sketch of what it might look like to do option 3 with
FirebaseApp
(i.e.PlatformFirebaseApp
instead ofFirebaseAppData
).In the platform interface: