-
-
Notifications
You must be signed in to change notification settings - Fork 142
feat(firestore): add timestamp & fieldvalue management #677
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
base: main
Are you sure you want to change the base?
feat(firestore): add timestamp & fieldvalue management #677
Conversation
|
Hi @robingenz, For now, this PR is a prototype of the Timestamp & FieldValue management on Web and Android. Do you have some time to make early feedbacks on this before I continue the development? Especially on these points:
Thank you |
|
@mamillastre Thanks for the update. I'm currently on vacation and won't be back for another 2 weeks. I'll have a look at it as soon as i'm back. |
0335a02 to
cab4468
Compare
|
Hi @robingenz, |
|
Hi @mamillastre, I'll try to take a look at your PR over the next few weeks. The problem is that this is a complex feature that will probably keep me busy for several days. |
cab4468 to
21be503
Compare
|
I added all the timestamp & field values methods management for the web and android platforms. |
@capacitor-firebase/analytics
@capacitor-firebase/app
@capacitor-firebase/app-check
@capacitor-firebase/authentication
@capacitor-firebase/crashlytics
@capacitor-firebase/firestore
@capacitor-firebase/functions
@capacitor-firebase/messaging
@capacitor-firebase/performance
@capacitor-firebase/remote-config
@capacitor-firebase/storage
commit: |
|
Note on iOS: the solution to use the "toJSON()" method to serialize the timestamps and field values do not work on iOS (unlike Android). Another solution must be found. For example:
These solutions can have a negative impact on the plugin methods performances depending on the document size. |
fc98ced to
5e4ca01
Compare
|
Is there an ETA on this? It looks like there's some pretty good progress, but I'm wondering if I should wait out or create a scrappy workaround for my particular use case. |
|
Hi @petermakeswebsites, I hope this feature can come with a 7.1 or 7.2 version. But there is no guarantee. You can test this feature right now using this dependency: npm i https://pkg.pr.new/capawesome-team/capacitor-firebase/@capacitor-firebase/firestore@677 |
01cdffb to
aeeb21d
Compare
|
This PR is now ready for review. Here is an example of use: import { FirebaseFirestore } from '@capacitor-firebase/firestore';
import { Timestamp } from "@capacitor-firebase/firestore/timestamp";
import { arrayRemove, arrayUnion, deleteField, increment, serverTimestamp } from "@capacitor-firebase/firestore/field-value";
const setDocumentWithTimestamp = async () => {
await FirebaseFirestore.setDocument({
reference: 'docs/timestamp',
data: {
now: Timestamp.now(),
fromDate: Timestamp.fromDate(new Date("2024-03-24")),
timestamp: new Timestamp(1711238400, 0)
},
});
};
const setDocumentWithFiledValues = async () => {
await FirebaseFirestore.setDocument({
reference: 'docs/field-value',
data: {
serverTimestamp: serverTimestamp(),
increment: increment(1),
fieldToDelete: deleteField(),
arrayUnion: arrayUnion('newItem1', 'newItem2'),
arrayRemove: arrayRemove('itemToRemove'),
},
});
};I separate these functionalities into their own modules to avoid to include the firebase web dependency in the main bundle. |
|
Hi @robingenz, do you have some time to make the code review? Thank you |
|
@mamillastre Yes, but I'm not happy with the PR yet. The Firebase JS SDK shouldn't be mandatory on Android and iOS. As soon as I have time, I'll try to come up with some alternative solutions. Until then, I'll leave this PR open for now. |
|
Thank you very much @robingenz. |
a612a83 to
b4f51c6
Compare
|
The branch has been rebased on the 7.3.0 version. |
|
Seems like there is good progress just wandering if @robingenz you will be able to prfioritize reviewing this or should i look for alternative time storage format. |
|
@mamillastre Any way to use this before it gets merged also. I can try it out in my project. |
|
@niteshgrg, But be careful, it will follow any updates we make in this pull request. |
|
@mamillastre There are also pre-releases for every single commit. |
I didn't know that. Very useful ! Thank you. For my last commit: |
|
Great thanks for this. Will incoorporate it :) |
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run changeset).Close #474, #443
Linked PR: #557
Usage example: