bufbuild / buf

The best way of working with Protocol Buffers.
https://buf.build
Apache License 2.0
9.18k stars 278 forks source link

google.protobuf.Value NullValue is reset to undefined fromPartial method of parent message #3481

Closed simonenkoav closed 3 days ago

simonenkoav commented 4 days ago

GitHub repository with your minimal reproducible example (do not leave this field blank or fill out this field with "github.com/bufbuild/buf" or we will automatically close your issue, see the instructions above!)

github.com/bufbuild/buf

Commands

buf generate

Output

fromPartial<I extends Exact<DeepPartial<Parent>, I>>(object: I): Parent {
    const message = createBaseParent();
    message.name = object.name ?? "";
    message.value = object.value ?? undefined;  // <--- value = null is replaced by undefined 
    return message;
}

Expected Output

I expect null value not to be replaced by undefined in fromPartial method.

Anything else?

We have proto message Parent like this:

message Parent {
    string name = 1;
    google.protobuf.Value value = 2;
}

and use buf generate, which generates this:

export interface Parent {
  name: string;
  value: any | undefined;
}

So the value field must be filled with builtin values (int, string, list, dict, null), which are, as far as I understand, are converted to Value

export interface Value {
  kind?:
    | { $case: "nullValue"; nullValue: NullValue }
    | { $case: "numberValue"; numberValue: number }
    | { $case: "stringValue"; stringValue: string }
    | { $case: "boolValue"; boolValue: boolean }
    | { $case: "structValue"; structValue: { [key: string]: any } | undefined }
    | { $case: "listValue"; listValue: Array<any> | undefined }
    | undefined;
}

in Parent.encode by Value.wrap when sending request to the server:

encode(message: Parent, writer: BinaryWriter = new BinaryWriter()): BinaryWriter {
    if (message.name !== "") {
      writer.uint32(10).string(message.name);
    }
    if (message.value !== undefined) {
      Value.encode(Value.wrap(message.value), writer.uint32(18).fork()).join();  // <--- here
    }
    return writer;
}

It works with all the types except null value — Parent.value is replaced by undefined and server receives empty field. And the problem seems to be in Parent.fromParent, which is called when sending request without our control and replaced Parent.value by undefined:

fromPartial<I extends Exact<DeepPartial<Parent>, I>>(object: I): Parent {
    const message = createBaseParent();
    message.name = object.name ?? "";
    message.value = object.value ?? undefined;  // <--- here
    return message;
}

before encoding.

Could you please support not replacing null values by undefined in fromPartial?

timostamm commented 3 days ago

Hey @simonenkoav, it looks like you are generating code with https://github.com/stephenh/ts-proto.

The buf CLI compiles Protobuf and hands descriptors (similar to an AST) to plugins that generate code. Anyone can write a Protobuf plugin that works with the buf CLI. ts-proto is one of the plugins that's commonly used to generate TypeScript. Can you file an issue with the upstream repository?

simonenkoav commented 3 days ago

Yes, you are right! Thank you, sorry for taking your time.