feat!: remove deprecated timestampInSnapshots setting #808
Conversation
googlebot
added
the
cla: yes
schmidt-sebastian
force-pushed
the
mrschmidt/deprecate
branch
2 times, most recently
from
ea59a77
to
7114340
Compare
Codecov Report
@@ Coverage Diff @@
## master #808 +/- ##
==========================================
+ Coverage 74.22% 74.24% +0.01%
==========================================
Files 50 50
Lines 2914 2904 -10
Branches 467 462 -5
==========================================
- Hits 2163 2156 -7
+ Misses 697 695 -2
+ Partials 54 53 -1
Continue to review full report at Codecov.
|
schmidt-sebastian
changed the title
schmidt-sebastian
force-pushed
the
mrschmidt/deprecate
branch
from
31045bd
to
959c41f
Compare
| // tslint:disable-next-line deprecation | ||
| settings.timestampsInSnapshots, | ||
| {optional: true} | ||
| ); |
There was a problem hiding this comment.
Am I reading the code correctly that we don't have any particular validation to prevent "extra" settings? (i.e. if a user typos 'projectId' as 'projectID' we'll just silently ignore it rather than let them know that they messed up?)
It would be nice if we had validation like the web SDK (https://github.com/firebase/firebase-js-sdk/blob/57a0f2bd78afba64b292125522f1071b386a920d/packages/firestore/src/api/database.ts#L181) and then now that we remove timestampsInSnapshots, people will get a very clear error if they're using it.
There was a problem hiding this comment.
No, we don't have this validation. The Server SDK supports a ton of options that are consumed by either our code, Google Gax or GRPC. I would be surprised if anyone knew the complete set of settings we support, hence I don't think we can validate them here.
The next release will be breaking (see #805). I would like to use this opportunity to clean up the code and remove timestampInSnapshots