Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate alternate accessor for fields with presence #182

Open
ksrnnb opened this issue Dec 30, 2023 · 3 comments
Open

Generate alternate accessor for fields with presence #182

ksrnnb opened this issue Dec 30, 2023 · 3 comments
Labels
enhancement New feature or request port-fix triaged Issue has been triaged

Comments

@ksrnnb
Copy link

ksrnnb commented Dec 30, 2023

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
+-- [email protected]
+-- [email protected]
`-- [email protected]
@dibenede
Copy link
Contributor

dibenede commented Jan 2, 2024

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).

@dibenede dibenede added the question Further information is requested label Jan 2, 2024
@ksrnnb
Copy link
Author

ksrnnb commented Jan 3, 2024

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
Copy link
Contributor

dibenede commented Jan 5, 2024

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.

@dibenede dibenede added triaged Issue has been triaged port-fix enhancement New feature or request and removed question Further information is requested labels Jan 5, 2024
@dibenede dibenede changed the title Support for optional field in toObject method Generate alternate accessor for fields with presence Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request port-fix triaged Issue has been triaged
Projects
None yet
Development

No branches or pull requests

2 participants