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

fix(firestore): timestamp values #557

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

fix(firestore): timestamp values #557

wants to merge 2 commits into from

Conversation

robingenz
Copy link
Member

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

@robingenz robingenz added platform: android Android platform bug/fix Something isn't working platform: ios iOS platform package: firestore labels Feb 14, 2024
@robingenz robingenz added this to the v5.5.0 milestone Feb 14, 2024
@robingenz robingenz self-assigned this Feb 14, 2024
@robingenz
Copy link
Member Author

robingenz commented Feb 14, 2024

@robingenz robingenz mentioned this pull request Feb 14, 2024
@robingenz robingenz modified the milestones: v5.5.0, v6.0.0 Feb 14, 2024
@robingenz robingenz removed this from the v6.0.0 milestone Apr 16, 2024
@mamillastre
Copy link
Contributor

Hi @robingenz, can I help you with this PR ?
I also need this feature.

Since we have to communicate with the native platforms throw a JSON format, I think #474 and #443 can be linked.

My idea is to pass to the native platform an object like:

import { TimeStamp, FieldValue } from "@capacitor-firebase/firestore";

{
   myTimestampField: TimeStamp.now(),
   myValueField: FieldValue.serverTimestamp()
}

That generates this JSON:

{
   "myTimestampField": {
      "_capacitorFirestoreFieldType": "timestamp",
      "_capacitorFirestoreFieldValue": {
         "seconds": 1736942400,
         "nanoseconds": 0
      }
   },
   "myValueField": {
      "_capacitorFirestoreFieldType": "fieldvalue",
      "_capacitorFirestoreFieldValue": {
         "method": "serverTimestamp"
      }
   }
}

We can imagine adding more features like these in the future with this same mechanism (like GeoPoint).

Downsides:

  • We have to loop over all the object to replace the fields on the native side.
  • When getting a document, we have to call a JS method to convert the native data back to the timestamps

What do you think of this approach.

@robingenz
Copy link
Member Author

@mamillastre I already had the same idea. Unfortunately, I haven't come up with a better solution yet.

When getting a document, we have to call a JS method to convert the native data back to the timestamps

However, it would be important to me that this step does not have to be carried out manually by the user, but that the plugin takes care of it. For this purpose, I would wrap all plugin calls on the web layer so that the data can be processed before and after the plugin call to the native layer. I also use a web layer like this for the Capacitor Bluetooth Low Energy plugin, for example, and it works very well.

This is how it works:

index.ts:

import { registerPlugin } from '@capacitor/core';

import { BluetoothLowEnergyClient } from './client';
import type { BluetoothLowEnergyPlugin } from './definitions';

const BluetoothLowEnergy = new BluetoothLowEnergyClient(
  registerPlugin<BluetoothLowEnergyPlugin>('BluetoothLowEnergy', {
    web: () => import('./web').then(m => new m.BluetoothLowEnergyWeb()),
  }),
) as any as BluetoothLowEnergyPlugin;

export * from './definitions';
export { BluetoothLowEnergy };

client.ts:

export class BluetoothLowEnergyClient implements BluetoothLowEnergyPlugin {
  private readonly plugin: BluetoothLowEnergyPlugin;

  constructor(plugin: BluetoothLowEnergyPlugin) {
    this.plugin = plugin;
  }

  public async connect(options: ConnectOptions): Promise<void> {
    // Do your thing
    this.plugin.connect(options);
  }
}

Feel free to create a PR. Let me know if you have any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/fix Something isn't working package: firestore platform: android Android platform platform: ios iOS platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(firestore): Timestamp values
2 participants