protoc-gen-ng: implement support for optional fields in proto3

[johannesf95]

Hi, first of all thanks for this great library! It's easy to integrate and really works like a charm within angular. 👍
Since proto3 added support for optional fields per default recently protobuf v3.15.0, it would be great to reflect it inside the generated classes instead of declaring every field as optional. IMHO printAsObjectMapping and printAsJSONMapping in the fields generators should only print the ?, if the field is explicitly declared as optional in the proto3 definition.
Additionally protoc throws this error currently, when trying to compile proto files with optional fields: call.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-ng hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.
It seems like protoc-plugin is causing the error, because it includes an on old protobuf compiler and has not been updated for quite a while:
https://github.com/konsumer/node-protoc-plugin/blob/331e33f49565be967007f3d817e99a5a94fb8a30/src/google/protobuf/compiler/plugin_pb.js
Is it still necessary to rely on this dependency or would it make sense to use google-protobuf/google/protobuf/compiler/plugin_pb directly, like improbable-eng/ts-protoc-gen does?

[smnbbrv]

Hi @johannesf95

nope, this old plugin generator is not anyhow something we should rely on. Of course it is better to migrate to one from google-protobuf.

I tried it so far, looks pretty good, the migration went quite quickly; looks like the old lib was sticking very close to the original protobuf messages.

Thank you very much for the time you took for the deep research!

[smnbbrv]

However, back to the original topic, question is, actually, whether there is a real need in optional parameters at all.

Optional parameters is something related to the transport in the runtime, I guess. On the TypeScript side the optional parameters mean something entirely different. All the scalar values get their defaults anyway, in other words they can never be null / undefined. The only case where something can be optional is the non-repeated message field.

Maybe it makes more sense to just have none of the optional fields at all, but the messages, and then respect this new flag in order to make them non-optional?

[johannesf95]

Hi @smnbbrv, thanks for this fast reaction and the change to a more future proof dependency stack! I really appreciate this, since we recently decided to use ngx-grpc in an enterprise project.
Sorry for not reading the grpc/proto3 specs carefully, but grpc is a new technology for me and I'm just gathering first experience on that complicated stack. 🤓 Your approach for handling optional makes sense and it is the same way how grpc-web d.ts files look like. Everything is required/available per default, but messages are optional when the flag is set.
I could create a PR, if you don't mind.

[smnbbrv]

I will try to summarize first, the first target is to have the following (without complexity that is introduced by optional):

1. single scalar types (including enums) are always not-null, because every scalar has a default value (according to protobuf)
2. single message types (including well-known types) are always optional (according to protobuf)
3. repeated fields are always not-null, because the initial value is empty array (according to protobuf)

If the flag optional is present, then... what should be changed?

As I see from proto3 field presence definition there are also some changes to the serialization / deserialization. The further difference is that it affects the JSON output format (toProtobufJSON in context of ngx-grpc) and the original protobuf message implementation gets has and clear methods, but this in particular cannot exist in ngx-grpc by design (and actually make no sence since one can assign null values directly). Some of this complexity, I hope, is taken by the protoc itself, this needs to be checked separately.

This could be quite a big, definitely a breaking change.

[smnbbrv]

Implemented by #57