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

support build options for filed type #69

Open
BeatsKitano opened this issue Aug 14, 2022 · 5 comments
Open

support build options for filed type #69

BeatsKitano opened this issue Aug 14, 2022 · 5 comments

Comments

@BeatsKitano
Copy link

This is a wonderful plugin! thank you for open source.
I have a little issue.this is my proto file hello.proto:

syntax = "proto3";

message Reply {
	string id = 1;  
}

this lib generate filed id type to string | undefined, but field id is define.
If id is optional, i think string | undefined is ok, now id is default required, the generate field type should be string, not string | undefined

version: latest

@ericwenn
Copy link
Member

@BeatsKitano This is intentional, as the default value fields (for example 0 on an int) may be omitted from the json serialization of protobuf message.

See https://developers.google.com/protocol-buffers/docs/proto3#json

@shuqingzai
Copy link

shuqingzai commented Feb 13, 2023

If the required option is set, undefined should not be required, right? Now there will be undefined whether it is set or not

syntax = "proto3";

import "google/api/field_behavior.proto";

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

@ericwenn
Copy link
Member

We've been discussing this internally, and landed on always setting type to T | undefined, as it will be consistent across proto versions and behavior.

For example REQUIRED might mean the value must be set on input, but does it require it to be set on output?

@shuqingzai
Copy link

shuqingzai commented Mar 14, 2023

  1. Perhaps you're right. Described at https://google.aip.dev/203
  2. It is again required when generating openapi documentation at protoc-gen-openapi
  3. There is no field to indicate that the response is required, which makes it very difficult for front-end personnel

@ericwenn
Copy link
Member

Regarding 3, there is a comment including the field behavior, for example:

// The resource name of the origin site of the shipment.
// Format: shippers/{shipper}/sites/{site}
//
// Behaviors: REQUIRED
originSite: string | undefined;

I'd be open to having build options for controlling this. Feel free to create a PR.

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