You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 (or google.protobuf.Timestamp) which accepts the value Date
When enabled, the generated types would use Date for any timestamp fields
When enabled, the protobufjs-wrapping code would override the default toObject and fromObject on the Timestamp class 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 #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:
the custom wrappers are defined on the protobufjs module globally, and aren't directly exposed by proto-loader
it is also possible to implement by augmenting the methods of the generated Timestamp class, but this also isn't directly exposed by proto-loader (they're on the protobufjs root).
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, fromObject could be backwards compatible and accept either the seconds/nsec object or Date, but toObject has 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!
The text was updated successfully, but these errors were encountered:
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.
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 JSDate
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
andbytes
which enables the generated code and types to preferDate
instances forTimestamp
-typed fields.Implementation-wise my suggestion is as follows:
timestamp
(orgoogle.protobuf.Timestamp
) which accepts the valueDate
Date
for any timestamp fieldsprotobufjs
-wrapping code would override the defaulttoObject
andfromObject
on theTimestamp
class to accept datesIf/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 #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:
toObject
/fromObject
implementations (mentioned in Automatically wrapping & unwrapping wrappers.proto types? protobufjs/protobuf.js#677)protobufjs
module globally, and aren't directly exposed byproto-loader
Timestamp
class, but this also isn't directly exposed byproto-loader
(they're on the protobufjs root).Date
s from toObject would likely need to be opt-in to keep remain backwards compatibleproto-loader
's typescript generation code would also need a flag to modify the generated types,fromObject
could be backwards compatible and accept either theseconds/nsec
object orDate
, buttoObject
has 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!
The text was updated successfully, but these errors were encountered: