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 for required fields in proto3 via options #1468

Open
LukeChannings opened this issue Aug 3, 2020 · 11 comments
Open

Support for required fields in proto3 via options #1468

LukeChannings opened this issue Aug 3, 2020 · 11 comments

Comments

@LukeChannings
Copy link

proto3 disallowed use of the required and optional rules and standard proto tooling (protoc, buf, etc.) will error if given a proto3 file that uses them.

I am compiling for a TypeScript project, and the fact that all fields are optional is a real hinderance to productivity.
I have to look at the proto file to figure out what's required and what isn't, because required fields are followed by a // required comment.

Looking at src/field.js this.required is only ever set if the required rule is present. Is it possible to set that based on an option too, so that I can have nice types but still be compliant with proto3?

@LukeChannings
Copy link
Author

LukeChannings commented Aug 4, 2020

An example project that provides validation via options is protoc-gen-validate.

Example:

Person x = 1 [(validate.rules).message.required = true];

Obviously there's no standard option that provides this functionality, but I'm not aware of any major reasons protobufjs can't use its own option for this functionality.

@marcobeierer
Copy link

Just a side note: proto3 has experimental support for optional fields since version 3.12.0. You can find detailed information in the field presence docs.

Thus fields now can be null and are not automatically initialized with the default value. Maybe treating all non-optional fields as required can work for you even if it's not a perfect solution.

However, I have not checked if protobufjs supports optional fields in proto3 yeat.

@vanthome
Copy link

Can someone clarify/ answer whether protobufjs supports this already?

@hulucc
Copy link

hulucc commented Nov 20, 2020

I need this too.

@mharsat
Copy link

mharsat commented Dec 9, 2020

@LukeChannings did you find a solution or workaround?
ts-proto has support for setting primitive fields as required, but it's lacking the verify function that protobufjs has for validation

@timostamm
Copy link

I would argue that all fields in proto3 are required, but probably not the way you want.

The language guide says:

When a message is parsed, if the encoded message does not contain a particular singular element, the corresponding field in the parsed object is set to the default value for that field.

protobuf.js does not implement this behaviour. For example, decoding from empty message data produces an object with no properties at all:

let f = Foo.decode(new Uint8Array(0));
console.log(f);
// Foo {}

Comparing with typescript implementations ts-proto and protobuf-ts (I am the author):

let f = Foo.fromBinary(new Uint8Array(0));
console.log(f);
// { a: '', b: 0, c: false }

The properties are initialized with their default values. When creating a new, empty message instance, the properties are initialized as well. This allows for a consistent typing with TypeScript:

type Foo = {
   a: string;
   b: number;
   c: bool;
}

I think the typing is an advantage, but it is important to note that this behaviour is probably not what most users expect from required fields.

If your API requires a name field to have a value in order to create a new user, for example, you still have to make sure to actually provide a non-empty string.

This was actually a design goal of proto3. From the language guide for proto2:

Required Is Forever You should be very careful about marking fields as required. If at some point you wish to stop writing or sending a required field, it will be problematic to change the field to an optional field – old readers will consider messages without this field to be incomplete and may reject or drop them unintentionally. You should consider writing application-specific custom validation routines for your buffers instead.

proto3 removes this problem. You can arbitrarily remove and add fields to a message, and older code will still be able to read and write valid messages.

If you want this capability and need mandatory fields at the same time, you have to resort to custom validation routines. I hope that protoc-gen-validate becomes a standard for this. Note that protobuf.js already exposes field options, so a validate() function is possible.

If you have full control over server and client, there is a simple alternative: You can label optional fields using the experimental proto3 optional label as mentioned by @marcobeier. Unfortunately, this doesn't seem to be possible with protobuf.js yet. verify simply returns null (valid), whether the field is optional or not.

I think it would be beneficial if protobuf.js would set default values for proto3 fields.
The question is whether the pretty big BC would be worth it...

@mharsat
Copy link

mharsat commented Dec 20, 2020

Thanks for the detailed response @timostamm.
I understand why proto3 eliminated required fields, in my case I only want the generated type definitions to further enforce the schema in typescript code, where limitations are different from proto. As @LukeChannings said, not having this gives a hard time for typescript devs.

I added support for a new required field option for proto files in my own fork. It doesn't affect the JS code, only the comments and the generated types. This allows defining both primitives and messages as required in the generated type definitions. For example,

syntax = "proto3";

message RicosContent {
    Document doc = 1 [required = true]; /// Document root
    Selection selection = 2; /// Last saved selection
    string version = 3 [required = true]; /// Ricos version used to create this schema
}

will generate

export interface IRicosContent {

    /** Document root */
    doc: IDocument;

    /** Last saved selection */
    selection?: (ISelection|null);

    /** Ricos version used to create this schema */
    version: string;
}

I hope protobuf.js will consider solving this. If this helps others I can suggest a PR.

@castarco
Copy link

castarco commented Apr 2, 2021

@mharsat I also implemented custom types to implement required fields (and flavored, branded & tagged types too) on our Typescript in our protoc plugin.

I see that there is a sort of (unofficial?) global registry for extensions & custom options ( https://github.com/protocolbuffers/protobuf/blob/master/docs/options.md ).

Maybe we should register a set of "standardized" extensions & custom options for Typescript? (instead of developing different nomenclature for each project) I see nothing specific for Typescript there.

@pulasthibandara
Copy link

For writing (encoding) types, I use this type lambda to extract types from the generated protobuf class. It uses keys of the interface to pick types from the class (which has the proper optional types) and returns a type that has required fields for all non-optional fields.

type InputType<C> = C extends new (arg: infer U) => infer I ? 
  I extends U ? Required<{ [K in keyof U]: I[K] }> : never :
  never

example:

message Person {
 string name = 1;
 uint32 string age = 2;
}
# given following protobuf gen.ts
interface IPerson {
  name?: (string|null);
  age?: (number|null);
}


class Person {
  constructor(properties?: IPerson);

  name: string;
  age: (number| null);
  ...
}

// Which you can use in a custom constcutor function.
const encode = <C extends new (args: any) => any>(cls: C) => (args: InputType<C>): C => new cls(args)

encode(Person)({
  name: 'john',
  age: null
})

@AceHack
Copy link

AceHack commented Aug 10, 2021

Does proto3 today now support optional and required?

@aksharp
Copy link

aksharp commented Sep 28, 2021

It would be great to be able to set Message type as required and enforce correctness at type level, rather than perform validation and copying into yet another type, especially in GC languages.

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

10 participants