protocolbuffers / protobuf-javascript

BSD 3-Clause "New" or "Revised" License
330 stars 66 forks source link

Generate alternate accessor for fields with presence #182

Open ksrnnb opened 6 months ago

ksrnnb commented 6 months ago

When I executed the toObject method on a message with a field marked optional, that field became the default value like 0 or "". I expect it would be undefined.

I guess it is because the third argument of getFieldWithDefault is not undefined. Since proto3 currently support optional keyword, when optional is specified, I think it should be undefined if it is not given.

proto

syntax = "proto3";

package com.book;

message OptTest {
    optional string name = 1;
}

generated toObject code

proto.com.book.OptTest.toObject = function(includeInstance, msg) {
  var f, obj = {
    name: jspb.Message.getFieldWithDefault(msg, 1, "")
  };

  if (includeInstance) {
    obj.$jspbMessageInstance = msg;
  }
  return obj;
};

versions

$ npm list --depth=0 -g
/usr/local/lib
+-- google-protobuf@3.21.2
+-- grpc_tools_node_protoc_ts@5.3.3
`-- grpc-tools@1.12.4
dibenede commented 6 months ago

Is your intent here to get a JSON representation of your message? We unfortunately do not support the proto3 JSON format.

In this library, toObject is something we recommend folks only use for convenience when writing tests and avoid using in production code because it uses its own special format (I don't know know the full history of it).

ksrnnb commented 6 months ago

I just want an object for the message, not a JSON representation, but I understand that toObject is for tests.

When I get a message and it's field, I should get it using getOptTest() and getName()? With this approach, it still seems to be an empty string, not undefined. When I get a field with optional, I want it to be undefined if it is unspecified.

Type of getName() generated by grpc_tools_node_protoc_ts is also string | undefined, so it would be nice to adapt to it.

export class OptTest extends jspb.Message {

    hasName(): boolean;
    clearName(): void;
    getName(): string | undefined;
    ...
dibenede commented 6 months ago

I'm grpc_tools_node_protoc_ts, so I can't speak to what it's doing.

Yes, basically you'll have something like:

const msg = OptTest.deserializeBinary(bytes)

msg.getName();  // will be default value for string, "", if absent

if (msg.hasName()) {
  // if need to distinguish between set/unset name field
}

Internally, we generate an alternate accessor for optional fields so that caller can get undefined, as you suggest, but it hasn't been released to open source.