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

findCustomEnumOption always returns 1 #534

Closed
jafaircl opened this issue Jul 25, 2023 · 6 comments
Closed

findCustomEnumOption always returns 1 #534

jafaircl opened this issue Jul 25, 2023 · 6 comments

Comments

@jafaircl
Copy link

jafaircl commented Jul 25, 2023

When using the findCustomEnum helper function in a plugin, the value returned is always 1. Specifically, I'm trying to set the google.api.field_behavior for a field like so:

message MyMessage {
  string id = 1 [(google.api.field_behavior) = IMMUTABLE];
}

And my plugin code looks like:

const fieldBehavior = findCustomEnumOption(field, 1052);
f.print("fieldBehavior: ", fieldBehavior, ",");

The expected value for this option would be 5. If the option isn't set, it returns undefined as expected.

I can see in the test cases that you are testing to make sure the value is returned. But, it just so happens that the value in your test case is 1.

Maybe I'm just misunderstanding what should be returned from this function? I thought it should be the field number from the enum. Should it not? If that's incorrect, how can I get the proper value?

Thanks!

Edit: Here is a minimum reproduction: https://github.com/jafaircl/enum-repro

Edit 2: It also doesn't seem to be a problem when parsing an option that is a message which contains an enum field. The message field in that case has the correct value. So if I did:

extend google.protobuf.MessageOptions {
  optional MyCustomOption message = 999999;
}

message MyCustomOption {
  enum MyEnum {
    MY_ENUM_UNSPECIFIED = 0;
    MY_ENUM_FIRST = 1;
    MY_ENUM_SECOND = 2;
  }

  MyEnum val = 1;
}

message MyMessage {
  option (repro.v1.message).val = MY_ENUM_SECOND;
}

That parses fine with findCustomMessageOption.

@smaye81
Copy link
Member

smaye81 commented Jul 25, 2023

Hi, @jafaircl this is because the google.api.FieldBehavior option is a repeated option. From our docs:

Note that repeated and map values are only supported within a custom message option. They are not supported as option types independently. If you have need to use a custom option that is repeated or is of type map, it is recommended to use a message option as a wrapper.

This is why your second edit worked -- because it uses a message option as a wrapper.

@jafaircl
Copy link
Author

Ok, thanks. I missed that part of the docs. Any idea when the protoc-gen-es might support custom options? That's really the only use case for our custom plugin at the moment.

@smaye81
Copy link
Member

smaye81 commented Jul 26, 2023

We've postponed this feature so far because of the issue that some users may not want their options shipped in a bundle, but with option retention in Protobuf v22.0 this becomes a bit easier. See this issue for more details.

TL;DR - it is on our roadmap for sure. Unfortunately no official timeline quite yet though.

@timostamm
Copy link
Member

@jafaircl, the issue here was that findCustomEnumOption() does not work for repeated fields. We just merged support for extensions, which provide a better and more convenient way to access the custom options in a plugin. See this PR for details: #669

@jafaircl
Copy link
Author

jafaircl commented Jan 19, 2024

@timostamm awesome. I see that you can get service and message extensions. Will we also be able to get field options? And will these values be available at runtime or only in plugins?

@timostamm
Copy link
Member

Yes, you can get field options as well, they are just extensions to google.protobuf.FieldOptions.

Options cannot be read at runtime yet. From the new docs:

At this point in time, it is not possible to retrieve custom options from generated code, since Protobuf-ES does not embed the full descriptors in the generated code.

Next up is better support for proto2 field presence. After that, embedding full descriptors in the generated code. The full descriptors will include options, which can then be read via extensions. So it'll be possible soonish - certainly this quarter.

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