firebase / firebase-admin-node

Firebase Admin Node.js SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
1.62k stars 368 forks source link

[Firestore] Types conflict between firebase-admin and @google-cloud/firestore #743

Closed whaatt closed 4 years ago

whaatt commented 4 years ago

Versions:

Steps to reproduce:

My library has @google-cloud/firestore@3.1.0 and firebase-admin@8.9.0 installed (so this is the latest version that has upgraded to Firestore 3). Consider the following snippet:

import { GeoPoint } from '@google-cloud/firestore'
import * as admin from 'firebase-admin'

const someModel = {
  location: new GeoPoint(0, 0)
}

const db = admin.firestore()
// ... snip ...
await db.collection('someCollection').add(someModel)

When running this snippet, I get an error with the following stack trace:

Error {
    message: 'Value for argument "data" is not a valid Firestore document. Detected an object of type "GeoPoint" that doesn\'t match the expected instance (found in field location). Please ensure that the Firestore types you are using are from the same NPM package.)',
}

Object.validateUserInput (node_modules/firebase-admin/node_modules/@google-cloud/firestore/build/src/serializer.js:303:15)
Object.validateDocumentData (node_modules/firebase-admin/node_modules/@google-cloud/firestore/build/src/write-batch.js:611:22)
CollectionReference.add (node_modules/firebase-admin/node_modules/@google-cloud/firestore/build/src/reference.js:1768:23)

If I replace the GeoPoint import with admin.firestore.GeoPoint (i.e. the GeoPoint class packaged with firebase-admin), this snippet works fine.

This appears to be a regression between the latest versions of the two libraries (firebase-admin@8.8.0 and @google-cloud/firestore@2.6.1 don't have this conflict).

I would consider using only firebase-admin instances of the Firestore types (if this is now necessary), but I would like to have confidence that there isn't a deeper issue.

Thanks in advance!

google-oss-bot commented 4 years ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

hiranya911 commented 4 years ago

Try importing GeoPoint from firebase-admin.

const someModel = {
  location: new admin.firestore.GeoPoint(0, 0)
}
whaatt commented 4 years ago

@hiranya911 Yeah, mentioned in the OP that this workaround fixes the issue. But unfortunately for pre-existing codebases where the two imports are mixed, this new error is a regression.

hiranya911 commented 4 years ago

In general types of these two libraries won't mix. That's why firebase-admin re-exports Firestore types in its own admin.firestore namespace. We've seen similar issues with other types in the past. If GeoPoint happened to work in the old versions, that's not by design.

schmidt-sebastian commented 4 years ago

We have update Firebase Admin to include @google-cloud/firestore at version 3. While we don't recommend or officially support mixing of types between admin.firestore and firestore, the usage pattern described in this issue should be possible again.