improbable-eng / ts-protoc-gen

Protocol Buffers Compiler (protoc) plugin for TypeScript and gRPC-Web.
Apache License 2.0
1.36k stars 173 forks source link

Bad TS types for proto3 optional fields #295

Open krpecd opened 2 years ago

krpecd commented 2 years ago

When a proto file has an optional field:

syntax = "proto3";

package example;

message Example 
{
  optional string optional_field = 1;
}

It generates this TS definitions

// package: example
// file: proto/example/example.proto

import * as jspb from "google-protobuf";

export class Example extends jspb.Message {
  hasOptionalField(): boolean;
  clearOptionalField(): void;
  getOptionalField(): string;
  setOptionalField(value: string): void;

  serializeBinary(): Uint8Array;
  toObject(includeInstance?: boolean): Example.AsObject;
  static toObject(includeInstance: boolean, msg: Example): Example.AsObject;
  static extensions: {[key: number]: jspb.ExtensionFieldInfo<jspb.Message>};
  static extensionsBinary: {[key: number]: jspb.ExtensionFieldBinaryInfo<jspb.Message>};
  static serializeBinaryToWriter(message: Example, writer: jspb.BinaryWriter): void;
  static deserializeBinary(bytes: Uint8Array): Example;
  static deserializeBinaryFromReader(message: Example, reader: jspb.BinaryReader): Example;
}

export namespace Example {
  export type AsObject = {
    optionalField: string,
  }
}

I think the namespace should be in this format:

export namespace Example {
  export type AsObject = {
    optionalField?: string,
  }
}

I can find out if the field is in the message by using hasOptionalField method. But TS check is not working as expected.

const optionalField: string = message.toObject().optionalField; // this should throw type Error
yzhaoa commented 1 year ago

toObject() for proto3 optional fields are still generated like so by the protoc js compiler:

    optionalField: jspb.Message.getFieldWithDefault(msg, 1, ""),

Note the Default part.

This conforms with the google-protobuf getter APIs and getter semantics in other languages. Please do not change this behavior with toObject().