Only set timestampsInSnapshots when explicitly requested via prop #42
Conversation
|
CI complains:
I'm not fully up to speed with how to mark a prop as optional within the conventions in use. Other than that I think this approach should be fine. |
tomsun
force-pushed
the
timestamps-in-snapshots-conditionally
branch
from
80b4010
to
ec64eb7
Compare
|
Got the warning locally in the console too, just didn't notice it at first... Dropping |
tomsun
force-pushed
the
timestamps-in-snapshots-conditionally
branch
from
ec64eb7
to
5847866
Compare
Codecov Report
@@ Coverage Diff @@
## master #42 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 6 6
Lines 86 89 +3
Branches 19 19
=====================================
+ Hits 86 89 +3
Continue to review full report at Codecov.
|
|
Adjusted expectations in tests to meet PR's suggested behavior |
|
@tomsun - I just merged a PR that upgraded to the new React context. It turns out that the PR also marked this prop as not required. However it is still being set in |
As of firebase@5.8.x timestampsInSnapshots is true implicitly green-arrow#39 Furthermore: calling settings() multiple times is not supported. If your app interacts with Firestore prior to using <FirestoreProvider>, FirestoreProvider's settings() call will get you into trouble: Uncaught FirebaseError: Firestore has already been started and its settings can no longer be changed. You can only call settings() before calling any other methods on a Firestore object
tomsun
force-pushed
the
timestamps-in-snapshots-conditionally
branch
from
5847866
to
2a44eea
Compare
|
@green-arrow sure! done |
|
Any plans on getting this merged to master any time soon? |
|
🎉 This PR is included in version 1.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
As of firebase@5.8.x
timestampsInSnapshotsseems to betrueimplicitly(example: #39 ) i.e. the corresponding prop and logic in react-firestore is unneccessary for anyone who is up-to-date. This PR keeps the prop around for backwards compatibility, but only acts on it when set explicitly.
Furthermore: another reason not calling settings() implicitly and unconditionally
within react-firestore: calling settings() multiple times is not supported.
If your app interacts with Firestore prior to using FirestoreProvider,
FirestoreProvider's settings() call will get you into trouble:
(This PR is possibly also a more graceful alternative to #37 )