Add defaults for message fields

[johnnysinger]

The proto file requires me to create my own classes or "message". When the .ts file is generated from the proto file all non primitive data types are defaulted optional. The .proto example:

message Timeframe{
     google.protobuf.Timestamp start = 1;
     google.protobuf.Timestamp end = 2;
     string name = 3;
     int32 duration=4;
}
message ProductionSummaryCreate{
     string workcellId = 1;
     optional string siteId=2;
     optional string createdBy=3;
     Timeframe timeframe=4;
}

The generated proto .ts file:

export interface Timeframe {
  start?: Date;
  end?: Date;
  name: string;
  duration: number;
}
export interface ProductionSummaryCreate {
  workcellId: string;
  siteId?: string | undefined;
  createdBy?: string | undefined;
  timeframe?: Timeframe;
}

In this case I need Timeframe and all its fields to be mandatory, when I try to create the GRPC controller I get this error:

[stephenh]

Hey @johnnysinger ; unfortunately Protobuf does not have a way to treat message fields as required (maybe it did in proto2?, but not in grpc/proto3).

What the Java-style protobuf output does is make Timeframe.start look "required" / not-optional, because it's never null, but really what happens is that if a client doesn't send start, then when decoded, start is set to a dummy/default Timestamp instance that has all of its fields set as zero.

I.e. the message isn't actually rejected as invalid / with a start is required runtime error which I kinda assume is what you want / what users expected "required" to mean.

That said, ts-proto could actually support the Java-style approach of "required", of creating a default Timestamp instance.

Personally I've not bothered doing that yet, b/c traditionally what I've wanted to do is enforce start as required by throwing a runtime error if it's not set, and that's actually easier to do with the current start? approach, by just checking for undefined, instead of it being start and then having to do something like if start === Timestamp.defaultInstance or something like that to check for "wait, the client didn't actually send this".

[stephenh]

For now renaming this to "Add defaults for message fields", which is how the other / Java / "canonical"-style protobuf outputs handle this situation. It seems pretty doable for ts-proto to support that, maybe as a feature flag.

Perhaps we could file another issue for "truly required fields", like that would throw runtime errors on missing fields; I personally like required fields enough that I think that would be a useful feature, but it's also sufficiently "not how protobuf works" that we should probably tackle it holistically/separately (i.e. even make primitive fields behave the same "not-optional now means required" way, I assume).

[stephenh]

We've had a lot of changes/flags added optional fields since this ticket was filed, and #1007 just added support for proto2 optional values, so going to kind of lazily assume this is taken care of. Can always reopen.