Repository navigation
Conversation
`firebase_core_platform_interface` could not be consumed outside of Flutter because everything it exposed lived in a single library importing `package:flutter/*`. Move the Flutter independent types (`FirebaseApp`, `FirebaseAppPlatform`, `FirebaseOptions`, `FirebaseException`, `FirebasePlatform`, `FirebasePlugin`, `defaultFirebaseAppName` and the shared exception helpers) into a new `firebase_core_common` package which the platform interface depends on and re-exports, so that a single class identity is kept for every consumer. `firebase_core_common` deliberately does not depend on the Flutter dependent Pigeon models: Pigeon's output stays untouched, and the conversion of the Pigeon options model into `FirebaseOptions` - which used to be the `FirebaseOptions.fromPigeon` constructor - moves to `firebaseOptionsFromPigeon` in `firebase_core_platform_interface`, next to the `MethodChannel` implementation which is its only user. As the platform interface can no longer reference `MethodChannelFirebase`, `FirebasePlatform.instance` does not default to it anymore: implementations register themselves (`FirebaseCoreWeb` from `registerWith`), `isRegistered` allows to check it, and `firebase_core` installs the MethodChannel implementation lazily on the first usage of the `Firebase` API. `firebase_core_web` now depends on `firebase_core_common` directly, so it can be used without a Flutter dependency. Neither package declares a `flutter` constraint in its `environment` anymore: pub does not select a package which declares one when Flutter is not available, so that constraint alone made both packages unreachable from pure Dart applications. The Flutter version a consumer needs is still enforced by `firebase_core` and `firebase_core_platform_interface`. `melos run check:pure-dart` compiles a Dart only entry point which uses `firebase_core_web`, so a Flutter library becoming reachable from it fails the build. The tests of `firebase_core_common` use `package:test` instead of `flutter_test`, so the package does not depend on Flutter at all. Part of firebase#18645 BREAKING CHANGE: `FirebaseOptions.fromPigeon` has been removed, because `FirebaseOptions` now lives in the Flutter free `firebase_core_common`, which must not depend on the Flutter dependent Pigeon models. Use the equivalent `firebaseOptionsFromPigeon` function, exposed by `firebase_core_platform_interface`, or build `FirebaseOptions` directly from the fields of the Pigeon response. `FirebasePlatform.instance` also does not default to `MethodChannelFirebase` anymore, so platform implementations must register themselves before the `Firebase` API is used. ## Migration Guide ### Replace `FirebaseOptions.fromPigeon()` **Before:** ```dart final options = FirebaseOptions.fromPigeon(response.options); ``` **After:** ```dart final options = firebaseOptionsFromPigeon(response.options); ``` ### Register the platform implementation Implementations of `FirebasePlatform` must install themselves (`FirebasePlatform.instance = MyFirebasePlatform()`) from their plugin's `registerWith`, as `firebase_core` (`MethodChannelFirebase`) and `firebase_core_web` (`FirebaseCoreWeb`) do.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request extracts pure Dart types and platform interfaces from firebase_core_platform_interface into a new package firebase_core_common, allowing Firebase Core platform implementations to be consumed outside of Flutter. Corresponding dependencies and imports have been updated across firebase_core, firebase_core_platform_interface, and firebase_core_web. Feedback on the changes includes a recommendation to make the constants parameter nullable in registerConstantsForApp to prevent potential runtime errors, and a suggestion to avoid using the ! operator in firebase.dart by utilizing flow analysis with a local variable, in compliance with the repository style guide.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…nterface/platform_interface_firebase_plugin.dart Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
kevmoo
left a comment
kevmoo
left a comment
There was a problem hiding this comment.
Please hold off on further changes here while we finish aligning with the maintainers on the broader #18645 design, which overlaps directly with this PR:
- Package name, location, and library split (
package:firebase_common): Rather than a core-onlyfirebase_core_commonpackage, we are planning a single pure-Dart leaf packagepackage:firebase_common(atpackages/firebase_common) that also hosts shared Firestore value types (Timestamp,GeoPoint,VectorValue,Blob,FieldPathinpackage:firebase_common/firestore.dart) acrossflutterfire,firebase-admin-dart, andfirebase-functions-dart. It also separates pure domain models (package:firebase_common/firebase_common.dart) fromplugin_platform_interfacecontracts (package:firebase_common/platform_interface.dart) so server packages do not pull inplugin_platform_interface. - Moving
FirebasealongsideFirebaseApp:cloud_firestore_web.dartandfirebase_auth_web.darttakeFirebaseAppand callFirebase.app(). LeavingFirebaseinpackage:firebase_corestill forces other*_webpackages to depend onpackage:firebase_core(Flutter). - Pigeon conversion & repo tooling: Prefer an extension (
extension CoreFirebaseOptionsX on CoreFirebaseOptions { FirebaseOptions toFirebaseOptions() }) infirebase_core_platform_interfaceover a top-levelfirebaseOptionsFromPigeonfunction, add the direct dependency tofirebase_core/pubspec.yaml, and updatescripts/bom_analysis.dartandscripts/generate_bom.dartto tolerate workspace packages without anenvironment.flutterkey.
Moving FirebaseApp into the pure-Dart package and lazily registering MethodChannelFirebase in Firebase._delegate are both directionally aligned with where we want to go—we'll follow up on #18645 and this PR once the package layout is locked down so you don't have to do throwaway rework.
|
I'm closing this for now. We ended up calling our flutter side app directly from the jaspr client once it is loaded instead. |
2 participants
Description
Move the Flutter independent types into a new
firebase_core_commonpackage which the platform interface depends on and re-exports, so that a single class identity is kept for every consumer.Breaking change:
firebase_core_commondoes not depend on the Flutter dependent Pigeon models: Pigeon's output stays untouched, and the conversion of the Pigeon options model intoFirebaseOptions- which used to be theFirebaseOptions.fromPigeonconstructor - moves tofirebaseOptionsFromPigeoninfirebase_core_platform_interface, next to theMethodChannelimplementation which is its only user.An alternative would have been to move the flutter independent models, after generation. This avoid a breaking change, but in this case i believe the breaking change is niche (
fromPigeon), not worth the complexity cost of splitting the output after gen.Before merging this I believe that strategy should be investigated accros other packages, to see whether the breaking change or alternative is better suited (in particular in regards to firestore).
As the platform interface can no longer reference
MethodChannelFirebase,FirebasePlatform.instancedoes not default to it anymore: implementations register themselves (FirebaseCoreWebfromregisterWith),isRegisteredallows to check it, andfirebase_coreinstalls the MethodChannel implementation lazily on the first usage of theFirebaseAPI.firebase_core_webnow depends onfirebase_core_commondirectly, so it can be used without a Flutter dependency. Neither package declares aflutterconstraint in itsenvironmentanymore: pub does not select a package which declares one when Flutter is not available, so that constraint alone made both packages unreachable from pure Dart applications.Part of #18645
Migration Guide
Replace
FirebaseOptions.fromPigeon()Before:
After:
Register the platform implementation
Implementations of
FirebasePlatformmust install themselves (FirebasePlatform.instance = MyFirebasePlatform()) from their plugin'sregisterWith, asfirebase_core(MethodChannelFirebase) andfirebase_core_web(FirebaseCoreWeb) do.Related Issues
#18645
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.yamland 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?