grpc-node
grpc-node copied to clipboard
Seamless Timestamp/Date encoding/decoding
Is your feature request related to a problem? Please describe.
Currently dealing with google.protobuf.Timestamp types is rather clunky if you want to interop native JS Date instances. Custom code must be used upon reading and writing messages to translate.
Describe the solution you'd like
Introduce an option similar to longs and bytes which enables the generated code and types to prefer Date instances for Timestamp-typed fields.
Implementation-wise my suggestion is as follows:
- Add a new option called
timestamp(orgoogle.protobuf.Timestamp) which accepts the valueDate - When enabled, the generated types would use
Datefor any timestamp fields - When enabled, the
protobufjs-wrapping code would override the defaulttoObjectandfromObjecton theTimestampclass to accept dates
If/when protobufjs gains an option to do native Dates, the third bullet could be replaced with that (or I could try and get something landed with them first, before pursing the type-generation change here).
I'm happy to implement this myself and submit a PR, provided I a bit of steer on whether this would go anywhere.
Describe alternatives you've considered
I note that this was previously asked for in https://github.com/grpc/grpc-node/issues/357 and closed, but the ecosystem has progressed since then.
I've done a bunch of code spelunking today and here's what I think the current state of things is:
- protobufjs v6 gained support for custom wrappers, which can extend the default
toObject/fromObjectimplementations (mentioned in https://github.com/protobufjs/protobuf.js/issues/677) - the custom wrappers are defined on the
protobufjsmodule globally, and aren't directly exposed byproto-loader - it is also possible to implement by augmenting the methods of the generated
Timestampclass, but this also isn't directly exposed byproto-loader(they're on the protobufjs root). - There is an old protobuf.js PR to add a wrapper native Date support, but this appears to have stalled and it's focused on JSON encoding anyway [https://github.com/protobufjs/protobuf.js/pull/1258]
- Even if that PR was responded to by maintainers and landed, returning
Dates from toObject would likely need to be opt-in to keep remain backwards compatible proto-loader's typescript generation code would also need a flag to modify the generated types,fromObjectcould be backwards compatible and accept either theseconds/nsecobject orDate, buttoObjecthas to pick a side.
If there's another way to make this all interop nicely with less work, or without making changes to core I'd be happy to take that approach instead!
Any thoughts on a way forwards here? Or should I be starting with a petition to protobuf.js?
I'm sorry for not responding earlier. I saw this but it dropped off my radar over Christmas. My main concern here is that proto-loader is currently a mostly-transparent wrapper around protobufjs, especially in terms of serialization and deserialization behavior. With this change, it would start to have its own serialization behavior and options, which would increase complexity and the risk of future incompatibilities with protobufjs.
Yeah, that makes a lot of sense to me.
I'll look into taking this up with them