Timestamp to String conversion

[actuosus]

There is should be an option to convert Timestamp as a string.

[stephenh]

I'm good with this as a config option but would ask for:

• add a new integration/<pick-some-name> directory with a parameter.txt with the new option and include the generated output in the PR. This will ensure the output compiles and will serve as a regression test
• Right now you're only changing the type, and not handling any of the protobuf (encode & decode) or JSON (toJSON / fromJSON) serialization/deserialization.

Can you add tests (within the new integration/<pick-some-name> directory) that show serialization/deserialization with the string-based timestamps works correctly?

[stephenh]

Hm. Given we already have forceLong which can be long or string, wonder if we should have (the newly added) useDate be something like builtin (to use Date) / string (as you propose) / protobuf (the google.protobuf.Timestamp type as-is).

I don't really love those names (builtin / string / protobuf)...ideally could think of better names.

But the main idea would be to reuse the existing useDate flag, by turning it into a tri-state.

Maybe just:

• useDate=true -- default behavior, uses Date
• useDate=false -- as-is today, uses `google.protobuf.Timestamp
• useDate=string -- as you want to add

[actuosus]

Yeah. Let's stick with the tri-state option. I'll change the code accordingly.

[jessebutterfield]

What's going on with this change? I'd love to see this functionality added as it allows me to align my interfaces directly with the json I get over the wire without any decoding/encoding. Seems like there has been a lot of drift since this request was opened. I'm happy to help finish this off. Would it be easier to start with a fresh branch and just try cherry pick the relevant changes?

[stephenh]

Would it be easier to start with a fresh branch and just try cherry pick the relevant changes?

Yep, due to the ts-poet upgrade there was a significant amount of churn/conflicts.

json I get over the wire without any decoding/encoding

That's an interesting goal. I think I wanted to do that as one point too...i.e. const message = JSON.parse(string) as Message.

I think the wrinkle would be primitives that are default values, i.e. if message.firstName = "" then depending on who is encoding / sending the JSON (i.e. a go server or what not), the firstName key might be left entirely out of the JSON.

So you might end up with code doing message.firstName.length > 1 being type-safe based on the Message.firstName: string type, but that didn't actually come back from JSON.parse.

And filling in these defaults is essentially what Message.fromJSON does.

Granted, if you control the server, you could tell it to always include these primitive values...

I think as happenstance ts-proto itself currently does include firstName: "" in Message.toJSON although that could change. I suppose if you did JSON.stringify(message) that would also include the firstName: "".

[jessebutterfield]

Yep, due to the ts-poet upgrade there was a significant amount of churn/conflicts.

Cool. I'll open a new branch and try to get this working. Thanks for being so responsive.

I think the wrinkle would be primitives that are default values, i.e. if message.firstName = "" then depending on who is encoding / sending the JSON (i.e. a go server or what not), the firstName key might be left entirely out of the JSON.

We have a rule against using unwrapped primitives, so not a problem for our use case. The only default we have to make sure to send from the server is default enums.

[stephenh]

This functionality was just merged as part of #221.