[REQUEST] Remove extra layer of nesting for oneof fields

[jcready]

As far as I can tell in all other reference implementations the identifier that comes after oneof in the proto files is never referenced. For example:

message SampleMessage {
  oneof test_oneof {
    string name = 1;
  }
}

message SampleRequest {
  SampleMessage message = 1;
}

message SampleResponse {}

service SampleService {
  rpc SampleMethod(SampleRequest) returns (SampleResponse);
}

The reference C++ code

SampleMessage message;
message.set_name("name");

But the protobuf-ts generated code requires you to reference test_oneof.

const client = new SampleService();
const {response, status} = await client.sampleMethod({
  message: {
    testOneof: { // unnecessary layer of nesting 
      oneofKind: 'name',
      name: 'test'
    }
  }
});

It would be great to remove the requirement to reference the oneof identifier like the other reference implementations.

[timostamm]

The oneof identifier is used in C++. An enum type and a getter is generated. This can be used to inspect which oneof field is selected (or whether none is selected). Most importantly, if you set a oneof field in C++, the sibling fields are automatically reset.

Because protobuf-ts uses plain java objects, we cannot reset fields automatically. Consider this proto:

message SampleMessage {
  oneof test_oneof {
    string name = 1;
    int32 age = 2;
  }
}

Flattening all oneof fields would allow the following instance:

let x: SampleMessage = {
  name: "foo",
  age: 123
};

Which is ambiguous. With the algebraic data type, you get a compile error if you try the same.

If protobuf-ts would generate classes instead of interfaces then yes, oneof fields should not be nested. Oneof integrity could simply be taken care of with setters.

But generating classes hugely increases code size. Interfaces are stripped in JavaScript and we only need some metadata. For example, protobufjs generates ~136kb where protobuf-ts generates ~42kb. See code size vs speed.

The drawback is added boilerplate. When consuming a message, it's not so bad:

https://github.com/timostamm/protobuf-ts/blob/54d5e99266a87146cf1ed46b37d8fea9e7c74e33/packages/example-chat-system/client.ts#L34-L47

Producing a message obviously requires more typing:

https://github.com/timostamm/protobuf-ts/blob/54d5e99266a87146cf1ed46b37d8fea9e7c74e33/packages/example-chat-system/server.ts#L54-L60

Proto: https://github.com/timostamm/protobuf-ts/blob/54d5e99266a87146cf1ed46b37d8fea9e7c74e33/packages/example-chat-system/protos/service-chat.proto#L20-L27

So far, I haven't found a better way to type oneofs. But I am open for suggestions.

[jcready]

I wonder if perhaps instead of nesting with a single oneofKind sigil that instead a sigil could be created per oneof identifier. And since all fields are camelCase an underscore prefix could prevent collisions.

 // print all chat events 
 call.response.onMessage(message => { 
     switch (message._event) { 
         case "joined": 
             term.print(`* ${message.joined}`); 
             break; 
         case "left": 
             term.print(`* ${message.left}`); 
             break; 
         case "message": 
             term.print(`${message.username}: ${message.message}`); 
             break; 
     } 
 }); 

 await this.broadcast({ 
     username: user.username,
     message: request.message,
     _event: 'message',
 }); 

It would end up making the types a bit more verbose.

type ChatEvent = {
  username: string,
  message: string,
  _event: 'message'
} | {
  username: string,
  joined: string,
  _event: 'joined'
} | {
  username: string,
  left: string,
  _event: 'left'
};

[timostamm]

It would end up making the types a bit more verbose.

Perfectly acceptable. But what if you have 20 oneof groups in a message?

[jcready]

I believe you'd end up with the exact same output size if intersection types were used in place of interfaces (plus or minus a few parenthesis). But I suppose it's making TypeScript's job a lot harder than it needs to be. I've convinced myself this probably isn't worth it since every extra oneof exponentially increases the number of expanded intersections :)

https://github.com/timostamm/protobuf-ts/blob/54d5e99266a87146cf1ed46b37d8fea9e7c74e33/packages/example-chat-system/protos/service-chat.ts#L15-L47

/**
 * @generated from protobuf message spec.ChatEvent
 */
export type ChatEvent = {
    /**
     * @generated from protobuf field: string username = 1;
     */
    username: string;
    /**
     * @generated from protobuf oneof: event
     */
} & ({
    _event: "joined";
    /**
     * @generated from protobuf field: string joined = 2;
     */
    joined: string;
} | {
    _event: "message";
    /**
     * @generated from protobuf field: string message = 3;
     */
    message: string;
} | {
    _event: "left";
    /**
     * @generated from protobuf field: string left = 4;
     */
    left: string;
} | {
    _event: undefined;
});