-
Notifications
You must be signed in to change notification settings - Fork 149
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
Make UpdateData type Generic to fix lack of type safety when using withConverter() #1448
Comments
This would require |
Thanks for suggesting this. We are considering this for our next breaking change release (based on a similar suggestion here: firebase/firebase-js-sdk#4277) |
I think maybe this is causing a problem conflict between firebase-admin@10.0.1 and @google-cloud/firestore@5.0.1.
Seems like the TypeScript version for this package is already updated to support this. It always surprises me that all the different node Firebase packages have different implementations of the types for Firebase... |
@rgant Your error suggests that you are using two different versions of Firestore. Can you confirm whether your build is using more than one version? Unrelated to this, since we published 5.x, this issue can now be closed. We have updated our converter code to enforce type safety for |
I believe these are the latest versions of both tools: firebase-admin@10.0.1 - For Firebase Functions to access Firestore. (https://firebase.google.com/docs/functions/get-started) @google-cloud/firestore@5.0.1 - To setup a scheduled backup of Firestore. (https://firebase.google.com/docs/firestore/solutions/schedule-export) Both as per the documentation best I can tell. Seems odd these two tools aren't in sync with the types. Any advice where I should post this issue? To my eye, @google-cloud/firestore@5 types are less useful compared to firebase-admin@10. |
Can you post the output of "npm list"? |
Click to expand: `npm list`
|
You have both "@google-cloud/firestore@4.15.1" and "@google-cloud/firestore@5.0.1". Until firebase-admin pulls in 5.x, you should depend on "@google-cloud/firestore@4.15.1". |
That is listed as an optional dependency in firebase-admin's package.json. I wouldn't have expected that to cause issues. Seems like something in npm should warn me about these conflicts. Thanks! |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry for spamming, jamie has a v9 article here: https://plainenglish.io/blog/using-firestore-with-typescript-in-the-v9-sdk-cf36851bb099 |
I wanted to know why is this closed, I am using |
@ljrodriguez1 I think it's because UpdateData is now included / exported from firebase-admin since it upgraded its
So you can use that to type your data as described in this article by @JamieCurnow PS: Tantiantially related, does anyone here know if/when the firestore API from firebase-admin / @google-cloud/firestore will transition to the same "modular" API as the web SDK >=v9? |
@0x80 But that UpdateData does not support dot notation, and differs from the UpdateData implementation from the article. because of that when doing |
@ljrodriguez1 Ah ok that's good to know! I didn't fully understand your problem then. Maybe @schmidt-sebastian or @thebrianchen can chime in...? |
Is your feature request related to a problem? Please describe.
This package works great with Typescript when using the
.withConverter
method and enforces correct types when performing aget()
orset()
oronSnapshot()
, but fails to enforce types on theupdate()
method.With TS v4.1 we can now safely enforce that dot-notation strings as object keys are correct, so we can do better than:
Check it out...
Describe the solution you'd like
I propose that the helper types for creating a type that is able to handle object paths should be added to
FirebaseFirestore.UpdateData
. I think that a solution could look like this:Which would allow type safety like this:
Additional context
Proposed solution in action:
The text was updated successfully, but these errors were encountered: