Support google.api.field_behavior

[bastianeicher]

google.api.field_behavior provides a nice way to mark fields in Protobuf messages as optional or required.

For example:

string name = 1 [(google.api.field_behavior) = OPTIONAL];
Message message = 2 [(google.api.field_behavior) = REQUIRED];

I would like to propose an addition to ts-proto to handle these field options as follows:

• Fields with scalar types marked as OPTIONAL get treated like message types.

  name?: string;
  // -or-
  name: string | undefined;
  

• Fields with message types marked as REQUIRED get treated like scalar types.

  message: Message;
  

Does this sound good to you guys? If so, I could try creating a PR implementing this.

[DeClaessens]

Sounds good to me!

[stephenh]

I'm not against it, just worried/disappointed that there is yet-another-way-to-kinda-do-optional-ness in proto. In general it seems like a mess...

Reading the field behavior docs, it sounds like it does not affect anything on the wire, so my guess/assumption is that besides just changing the types like message: Message, the encode / fromJSON / fromPartial methods will all have to change slightly to somehow force-create/default Messages if that field is not send on the wire/json/object literal.

[jessebutterfield]

Reading the field behavior docs, it sounds like it does not affect anything on the wire, so my guess/assumption is that besides just changing the types like message: Message, the encode / fromJSON / fromPartial methods will all have to change slightly to somehow force-create/default Messages if that field is not send on the wire/json/object literal.

My thought had been to not force-create them and just assume all clients/servers are correctly ensuring the field is there. Though I know that messes with the whole always backwards compatible thing some people care very deeply about. Or if not, it would be nice if the google.api.field_behavior could be parsed into the FieldOptions or represented somehow in the unsupported options so that we could make use of it in a fork. But I couldn't follow exactly how the ts-proto-descriptors package is compiled.

[stephenh]

just assume all clients/servers are correctly ensuring the field is there

That is a fine assumption until it doesn't happen. :-) Re-reading the docs, it sounds like throwing an Invalid Argument error is expected behavior for this case, i.e. in decode / fromJson / fromPartial when a marked-required key is not set.

google.api.field_behavior could be parsed into the FieldOptions

Ah yeah, looks like that uses the extend syntax which ts-proto just doesn't support.

https://developers.google.com/protocol-buffers/docs/proto#extensions

Which, dunno, I really haven't thought about what that'd look like for ts-proto.

But I couldn't follow exactly how the ts-proto-descriptors package is compiled.

For now I'm just hand-compiling it and npm publish-ing it with the idea it rarely changes.

[jessmorecroft]

This for me would be awesome. I’ve taken a look at proto3 in detail only recently and am astounded that required messages are not supported. How is one able to reasonably communicate and enforce an API contract without this? Imagine if swagger didn’t support required types?! A mate of mine works for Google and when I questioned him on it he basically agreed and said you won’t be “prising proto2 out of his cold, dead hands”!

Anyway I figured I’d take a look out how Google defines its proto APIs and lo and behold there were it’s field option extensions that get around this! I think with support for these grpc would be a no-brainer for picking an interface tech (as opposed to swagger for example) but as it is right now it just feels like proto3 is a half way house of a protocol, which is a shame because grpc seems pretty awesome otherwise, with the http2/streaming support for example.

Anyway I was going to look into implementing this in ts-proto anyway but just saw this thread and thought I’d comment. I’m far from an expert here so please if anyone has any other suggestions for exposing an API with grpc (and required arguments!) please let me know!

[stephenh]

@jessmorecroft yeah, fwiw I'm definitely in your camp re general disappointment with how protobuf has handled optional/required over the years.

But otherwise, yeah a PR would be great, with the disclaimer I don't actually have any tips off the top of my head. Other than the previous comment that AFAIU this would require ts-proto supporting extensions, so that'd be like step 1.

(Although afaiu they're also only a proto2 feature now, and I generally anticipate most users of ts-proto are on proto3. It might actually be easier to move ts-proto back to using protobuf.js-built CodeGeneratorRequest/Response types to communicate with the protoc process, and stop using the ts-proto-descriptors package. That isn't as cool as being self-bootstrapped, but it's really a pretty minimal change IIRC from when I initially switched from protobuf.js --> ts-proto-descriptors. So undoing it shouldn't be too bad, if we really need extensions to do this.)

[jessebutterfield]

My understanding is that extensions are still supported by proto3 strictly for use in custom options. google.api.field_behavior is proto3 syntax and it's produced by google.

[jessmorecroft]

Thanks for the tip @stephenh. I’ll have a crack at some point.