fix: support firebase-admin v14 - #336
Conversation
v14 removed the legacy namespaced API from the root firebase-admin entry point, so `firestore(...)` was undefined and makeDocumentSnapshot threw. Move to the modular subpaths (getFirestore, DocumentReference, GeoPoint, Timestamp from firebase-admin/firestore; App, deleteApp, initializeApp, AppOptions from firebase-admin/app). The DocumentReference branch of objectToValueProto read a `_referencePath` internal that no longer exists, emitting `projects//databases//<path>`. It now reads projectId and databaseId off the Firestore instance and includes the missing `documents` segment. The old spec assertion could not catch this because DocumentReference.toString() returned "[object Object]" on the bundled @google-cloud/firestore. Drops admin ^8 and ^9 from peerDependencies. The modular subpaths have no exports entry before v10, and this package has already imported firebase-admin/firestore since v3.
![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 upgrades the 'firebase-admin' dependency to '^14.3.0' and migrates imports and usage from the legacy monolithic package to modular subpaths ('firebase-admin/app' and 'firebase-admin/firestore'). Additionally, it updates TypeScript configurations to skip library checks. Feedback on these changes suggests adding a fallback to 'process.env.GCLOUD_PROJECT' when resolving the 'projectId' from a 'DocumentReference' to prevent malformed reference paths if internal properties are unresolved.
| const projectId: string = | ||
| get(val, 'firestore.projectId') || | ||
| get(val, 'firestore._settings.projectId'); |
There was a problem hiding this comment.
If both firestore.projectId and firestore._settings.projectId are unresolved (for example, if internal properties change in a future release of firebase-admin), projectId will be undefined. This would result in a malformed reference path containing 'undefined' (e.g., projects/undefined/databases/...).
Adding a fallback to process.env.GCLOUD_PROJECT provides a safe guard, as this environment variable is typically initialized during test setup.
| const projectId: string = | |
| get(val, 'firestore.projectId') || | |
| get(val, 'firestore._settings.projectId'); | |
| const projectId: string = | |
| get(val, 'firestore.projectId') || | |
| get(val, 'firestore._settings.projectId') || | |
| process.env.GCLOUD_PROJECT || | |
| ''; |
The firebase-admin devDependency stays at ^12 because firebase-functions ^4.9.0 peer-requires ^10 || ^11 || ^12, so CI does not exercise v14 yet.
2 participants
Fixes #327.
What was broken. firebase-admin v14 removed the legacy namespaced API from the root
firebase-adminentry, sofirestore(...)wasundefinedandmakeDocumentSnapshotthrewTypeError: (0, firebase_admin_1.firestore) is not a function. The same import brokeobjectToValueProtoand the shipped.d.tsfiles.What changed. Moved to the modular subpaths (
firebase-admin/firestore,firebase-admin/app).deleteApp(app)replacesapp.delete(), which is absent from the modularApp. TheDocumentReferencebranch ofobjectToValueProtoalso read a_referencePathinternal that no longer exists, emittingprojects//databases//<path>; it now reads the ids off the Firestore instance.Verified. Build and suite green against admin 12.7.0, 13.10.0 and 14.3.0. The #327 reproduction runs against a packed tarball with the Firestore emulator, failing before and passing after.
Decisions. CI still only tests admin 12:
firebase-functions@^4.9.0peer-requires^10 || ^11 || ^12, so a v14 job needs the functions bump in flight oninlined.bump-functions-dep. The peer range also drops admin^8/^9, which the modular subpaths never supported. ThemakeDocumentSnapshotreturn type is left alone; annotating it breaks eightcloudevent/mocks/firestore/*files.