Embed descriptors in generated code #743
Conversation
…er, same as the proto3 test file
There was a problem hiding this comment.
The public API so far looks great. I left comments of my understanding of why/what the code is doing, let me know if I am off track somewhere.
The reflect API could infer a little more, but I am yet to look at the followup PR #745
| // TODO ServiceShape | ||
| // TODO MethodShape? | ||
|
|
||
| type MessageInit<T extends Message> = { |
There was a problem hiding this comment.
This is recursively removing the $desc and $typeName fields as they are not needed for the initializing a value.
There was a problem hiding this comment.
And it also makes all properties optional. It's a replacement for PartialMessage with a more fitting name.
Arguably, proto2 required fields should not be optional when creating a message (#414). We can consider generating an "input shape" attached to TypedDescMessage for messages using proto2 required, and fall back to MessageInit. This might come with a lot of complication (also for users), and required fields are heavily discouraged. Best to punt on this decision for now.
| O: O; | ||
| }; | ||
|
|
||
| class brand<A, B = unknown> { |
There was a problem hiding this comment.
This is for added typesafety the class is actually not used anywhere except to brand types. Equivalent to opaque in Flow.
It could have similar effects as instanceof but because it is at the type level users should be fine as long as the generated code doesn't conflict. Maybe it will impact WKTs but unsure at this point.
| }; | ||
|
|
||
| // TODO docs | ||
| export type MessageShape<Desc extends DescMessage> = |
There was a problem hiding this comment.
I am assuming the loose constraint of DescMessage as opposed to TypedDescMessage is to allow for dynamic descriptors.
There was a problem hiding this comment.
Yes, that always needs to be possible.
@srikrsna-buf, we're not out of the weeds yet. Besides the missing pieces (serialization, etc.) possibly surfacing issues, adding the
We might actually want to dial it down more for practical use cases. For an extreme example, it's possible to narrow down map key type and map value type, but it's absolutely awkward to work with. For now, map fields aren't fully implemented in reflect (what we got doesn't have test coverage). It's good enough to start implementing the missing pieces, and decide on the final details for reflect after dogfooding it a bit. |
This change allows us to provide custom options at runtime (#397), improve support for reflection-based operations, and to implement all edge cases of editions.
We decided to embrace the advantages of this change, and remove the intermediate layer of message types, enum types, and field information, for a much more simple system without classes. (Note that many details are subject to change - this PR is just the first step).
To be able to bootstrap the runtime until the change is complete, the updated runtime and plugin are added under the "next" moniker:
protoc-gen-es-nextis the updated plugin,@bufbuild/protobuf/nextis the updated runtime.