ProtoScript icon indicating copy to clipboard operation
ProtoScript copied to clipboard

Deserialize Timestamps (google.protobuf.Timestamp) into Date objects

Open tatethurston opened this issue 2 years ago • 3 comments

Today Protoscript deserializes timestamps into an object in the shape { seconds: bigint, nanoseconds: number }.

We could instead deserialize timestamps into JavaScript Date objects. The only unclear pieces are:

  1. JS Date objects lack some of the precision of the object form. Specifically, JS Date's can handle up to 3 digit of ms precision, vs the timestamp object's 9 digits of nanosecond precision.
  2. This would be a breaking change for user's source code. The wire format would be unchanged.

The above could be mitigated with an opt-in or opt-out flag. I'm inclined to make this the default behavior, and potentially consider an opt-out flag.

Altternatively, ProtoScript could create a subclass Timestamp from Date. That subclass could then be used anywhere a Date is used, and optionally provide the nanoseconds for code paths aware of Timestamp. Eg:

class Timestamp extends Date {
  #nanoseconds = 0;
  
  setNanoseconds(nanoseconds: number) {
    this.#nanoseconds = nanoseconds;
  }
  getNanoseconds() {
    return this.#nanoseconds;
  }
}

Or something more clever using the constructor arguments -- but I'm considered about edge cases and maintenance there.

Please comment or like this issue if you're a user and want to see this change land.

tatethurston avatar Oct 18 '23 21:10 tatethurston

Some prior art: https://github.com/bufbuild/protobuf-es/blob/main/docs/runtime_api.md#timestamp

I think the helper approach also has some benefits because automatically converting to Date is lossy, like you mentioned.

But I do think that for clients who know the tradeoff, opting into automatic Date serialization is the best DX.

noahseger avatar Oct 24 '23 14:10 noahseger

Yeah agreed that automatic deserialization to Date is the best DX -- I'm inclined to make that the default. And then I'm open to supporting a config option to disable deserialization to Date, though I might wait until someone requests that. I think it's unlikely that JS users will be looking to use protobuf timestamps with granularity smaller than 1ms, but I'm open to being wrong.

tatethurston avatar Oct 25 '23 03:10 tatethurston

@noahseger if you’re interested and have time to take a swing at this I’m happy to review it

tatethurston avatar Oct 25 '23 22:10 tatethurston