[go: up one dir, main page]

Skip to content
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

feat(firestore): add timestamp & fieldvalue management #677

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mamillastre
Copy link
Contributor

Pull request checklist

Please check if your PR fulfills the following requirements:

  • The changes have been tested successfully.
  • A changeset has been created (npm run changeset).
  • I have read and followed the pull request guidelines.

Close #474, #443

Linked PR: #557

Usage example:

import { FirebaseFirestore, Timestamp, serverTimestamp } from "@capacitor-firebase/firestore";

FirebaseFirestore.setDocument({
    reference: "collection/doc",
    data: {
      timestamp: Timestamp.now(),
      fieldValue: serverTimestamp()
    },
 });

@mamillastre
Copy link
Contributor Author

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:

  • You advise me here to manage the data parsing from the native layer like the Bluetooth Low Energy Plugin. But I have to implement all the plugin methods, even those that are out of scope.
    Here, I use a Proxy. Tell me if you are ok with this solution.
  • When using the plugin, the firebase dependency is mandatory for the Timestamp & FieldValue usage. But I think you prefer avoiding this.
    In this case, I see two possibilities:
    1. Copying the Timestamp code from the firebase js SDK into the plugin
    2. Set the Timestamp implementation in a separated index.js file to avoid to install the firebase dependency if the timestamp is never used

Thank you

@robingenz
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(firestore): Timestamp values
2 participants