-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
fix(firestore, types): allow FieldValues, Date and Timestamp in doc set and update #5901
fix(firestore, types): allow FieldValues, Date and Timestamp in doc set and update #5901
Conversation
This pull request is being automatically deployed with Vercel (learn more). react-native-firebase-next – ./website_modular🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/D9mD8xtQ31vHnakkpXipt8boBxJr react-native-firebase – ./🔍 Inspect: https://vercel.com/invertase/react-native-firebase/FBNoDvWXYhU2yEBzxtSRQEHu6JqJ |
Codecov Report
@@ Coverage Diff @@
## main #5901 +/- ##
=============================================
+ Coverage 24.61% 52.62% +28.01%
- Complexity 0 620 +620
=============================================
Files 97 208 +111
Lines 4230 10154 +5924
Branches 1026 1612 +586
=============================================
+ Hits 1041 5343 +4302
- Misses 2587 4553 +1966
+ Partials 602 258 -344 |
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 LGTM after review (and note upstream types are here https://github.com/firebase/firebase-js-sdk/blob/master/packages/firestore-types/index.d.ts though we are a little divergent and perhaps they are either not handling this well or are handling it via any
-ification)
I've seen this myself though and your test lends me confidence. I have a medium success ratio track record with type changes, sometimes a tiny change even intended to be non-breaking results in something else popping up from someone else, so we'll have to keep eyes/ears open post release. Should be fine though? 🤞
Description
This is a fix for Typescript types to remove the following two errors:
Related issues
Release Summary
fix(firestore): Adjust types to allow setting FieldValues and allow Date in place of Timestamp
Checklist
Android
iOS
e2e
tests added or updated inpackages/\*\*/e2e
jest
tests added or updated inpackages/\*\*/__tests__
Test Plan
Run tests.