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

Field.optional should not be !Field.required for proto3 #1850

Open
egamble opened this issue Jan 17, 2023 · 3 comments
Open

Field.optional should not be !Field.required for proto3 #1850

egamble opened this issue Jan 17, 2023 · 3 comments

Comments

@egamble
Copy link

egamble commented Jan 17, 2023

protobuf.js version: 7.1.2

We are parsing a .proto file in proto3 format with protobufjs.loadSync. Using the resulting parsed objects, we need to distinguish between a field that is marked optional vs. a field that is not marked optional and also not marked required. Unfortunately, the protobufjs parser always sets Field.optional to the value of !Field.required, so we can't detect the optional keyword in the parsed Field object.

We use the Golang protobuf library with the same .proto file (in proto3 format) which generates different Golang code for fields marked optional, vs. fields not marked as either optional or required. We need to use the protobufjs parser in a similar way.

For example, we would like to detect the difference between the parsed field1 and field2 objects in this .proto file:

syntax = "proto3";

message Message1 {
  string field1 = 1;
  optional string field2 = 2;
}
@jonaskello
Copy link

I encountered the same problem, the parser ignores the optional keyword. Is there a work-around?

@egamble
Copy link
Author

egamble commented Aug 20, 2024

I encountered the same problem, the parser ignores the optional keyword. Is there a work-around?

Haven't found one.

@martin-traverse
Copy link
Contributor

Hi - I did some work related to this in PR #2011 to fix nullability in the type info for TS and JS Doc. The short answer is that for proto3 you probably want this test:

var is_optional = field.options != null && field.options["proto3_optional"] === true;

For a more complete answer, rather than thinking of optional / required, I found it more helpful to think of "implicit" and "explicit" presence as described in the new Protobuf editions spec. The optional keyword in proto3 indicates explicit presence, but implicit presence fields are still optional and can be omitted on the wire, so setting field.optional = false would not be accurate and would probably cause a lot of problems. Only proto2 has fields that are actually required.

In cli/targets/static.js I added a couple of methods, isExplicitPresence() and isImplicitPresence(), which work for both proto2 and proto3 syntax. You could make a similar method isLegacyRequired(). Ideally these methods should be updated to work with the new editions syntax qualifiers as well.

For reference: https://protobuf.dev/editions/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants