Consider making $type optional when generating the type registry

[davidzeng0]

It'd be cumbersome to type

Message.encode({
  $type: "...",
  submessage: {
    $type: "..."
  }
})

I'll gladly PR this if approved

[stephenh]

Ah sure, that sgtm. Maybe like an encode-specific mapped type that removes the $type field?

[davidzeng0]

My idea was sort of to make the type field $type?: "..."

because then writing

const message: Message = {
  ...
}

also works.

I've checked to make sure making the field optional does not break anything, and it doesn't.
If users expect a $type field, they can expect it to always exist after decoding, and if they need it for encoding
they will have to specify it themselves anyway.

[stephenh]

My concern about $type? is that then readers would not be able to rely on it always being set, which seems like it would materially affect the usefulness of the $type field.

We already have the fromPartial methods that take subsets of the message, and so I'd probably nudge to do something like that...i.e. a modified/mapped type (either for fromPartial, just use the existing DeepPartial type, and maybe update encode to accept it as well) that is only for creation of Messages.

But then internally within fromPartial / encode, we make sure the $type field is always set, so that then readers/callers can always rely on it's availability/non-optionalness.

[davidzeng0]

I see. I might add an option like initializeFieldsAsUndefined=false but for fromPartial because I do need some fields to not be initialized rather than an empty value. What do you think about that?

[stephenh]

Hm, isn't it supposed to be non-observable whether a field was not-initialized or initialized-as-empty? What sort of fields do you need to stay non-empty?

[davidzeng0]

That's only non-observable on proto3, but I am using some very backwards compatible services that read the presence of a field in proto2.

[stephenh]

Ah I see...yeah, if it's not too disruptive to the code generation, I think a flag for that behavior sgtm.

Afaiu that would let us close out #139 ? Which would be pretty great if so.

[davidzeng0]

I feel like that issue is already fixed? I'm not sure if I'm missing anything but it sounds like they just want useOptionals=all

[davidzeng0]

Fixed in #811