Closed ngbrown closed 3 months ago
Hey Nathan,
we don't export MessageInit
for forward compatibility. Imagine that we want to accept field representations in create()
that cannot be inferred from the generated type. MessageInitShape
allows us to attach a separate type for init to the descriptor, and use it for create()
.
We expect that you would typically want to write a generic function that works with arbitrary messages. We have an example in the v2 manual:
import { getOption, type DescMessage, type MessageShape } from "@bufbuild/protobuf";
import { reflect } from "@bufbuild/protobuf/reflect";
import { sensitive } from "./gen/example-option_pb";
export function redact<Desc extends DescMessage>(
schema: Desc,
message: MessageShape<Desc>,
) {
const r = reflect(schema, message);
for (const field of r.fields) {
if (getOption(field, sensitive)) {
// This field has the option (example.options.sensitive) = true
r.clear(field);
}
}
}
[!TIP]
EnumShape
extracts the enum type from an enum descriptor.MessageShape
extracts the type from a message descriptor.MessageInitShape
extracts the init type from a message descriptor - the initializer object forcreate()
.
The function could also take an init object with MessageInitShape<Desc>
. For this pattern, the types are pretty concise - MessageInit<MessageShape<Desc>>
would actually be more verbose.
But your function is a different case, it's specific to one message. I'd love to better understand the use case. What kind of function are you using it for?
Hi Timo,
Thanks for explanation. I'm not sure I totally understand your reasoning though. The message descriptor is really doing a lot because it also has the JSON type reference (when the json_types=true
option is turned on). I also get errors because the MessageInitShape
is doing too much. See my examples below.
Since filing this I turned on the JSON generated types because in my application, the Node.js backend-for-frontend has to communicate with the front-end by serializing via JSON. I had to turn it on because of bigint
, but I see that enums also change. I'm using Remix, so while the current version still needs this, the future version will communicate with turbo-stream, so I can use bare JS types again, so I might drop the json conversion at that point.
That is some background on the application, but for the specific use case where I would use MessageInit
, there's actually two situations.
First scenario is where a UI component would normally be initialized with a message, but if that isn't available, or I'm manipulating the values for display (and pending UI), I want to just manipulate the relevant values without worrying about the $typeName
value:
export type IProtoGearRatioLimit = MessageInit<ProtoGearRatioLimit>;
export type RequiredNonNullable<T> = { [P in keyof T]-?: NonNullable<T[P]> };
export type GearRatioLimit = RequiredNonNullable<IProtoGearRatioLimit>;
const [gearLimits, setGearLimits] = useState<GearRatioLimit[]>([
{ gear: 1, highLimit: 207.1, lowLimit: 121.8 },
]);
When I use export type IProtoGearRatioLimit = MessageInitShape<ProtoGearRatioLimitSchema>;
, then I get errors like this:
error TS2339: Property 'lowLimit' does not exist on type 'GearRatioLimit'. Property 'lowLimit' does not exist on type 'RequiredNonNullable
'.
and
error TS2339: Property 'lowLimit' does not exist on type 'GearRatioLimit'.
Property 'lowLimit' does not exist on type 'RequiredNonNullable'.
Second scenario is the return type of a bunch of my functions because I want to do a type union on whether a specific field is filled out or not. I have something like the following:
export type IProtoRpcRequest = MessageInit<ProtoRpcRequest>;
interface RequiredRpcRequestRouting {
standId: number;
type: ProtoRpcRequestType;
}
export type RpcRequestWithRouting = IProtoRpcRequest &
RequiredRpcRequestRouting;
Then functions that create the basics of a message:
export function createDataCollectionStartManualRecordCommand(
standId: number
): RpcRequestWithRouting {
return {
standId,
timestamp: getTimestamp(),
type: ProtoRpcRequestType.DATA_COLLECTION_START_MANUAL_RECORD_COMMAND,
dataCollectionStartManualRecordCommand: {},
};
}
Then finally, a function that needs a specific option if the specific fields to derive that information information are not filled out (or not available on that type):
export interface RpcClientFunc {
(
request: RpcRequestWithRouting,
options?: RpcClientFuncOptions
): Promise<ProtoRpcResponse>;
(
request: IProtoRpcRequest,
options?: RpcClientFuncOptions & { rpcTopic: string }
): Promise<ProtoRpcResponse>;
}
If I use export type IProtoRpcRequest = MessageInitShape<ProtoRpcRequestSchema>;
in this last scenario, I get errors like this:
Property 'standId' does not exist on type 'Record<string, unknown> | Message | MessageInit
| RpcRequestWithRouting'. Property 'standId' does not exist on type 'Message'.
and
error TS2345: Argument of type 'Record<string, unknown> | Message | MessageInit
' is not assignable to parameter of type 'ProtoRpcRequest | MessageInit | undefined'. Type 'Message' is not assignable to type 'ProtoRpcRequest | MessageInit | undefined'. Type 'Message' is not assignable to type 'ProtoRpcRequest'. Type 'Message' is not assignable to type 'Message<"hdi.vts.v1.ProtoRpcRequest">'. Type 'string' is not assignable to type '"hdi.vts.v1.ProtoRpcRequest"'.
Note: I used patch-package
to force the MessageInit
to be exported. Maybe that's why I got myself into this situation, but it also was the least change from the protobuf.js
and the protobuf-es
V1 implementation (with PlainMessage<Proto...>
).
I'm not sure I totally understand your reasoning though.
MessageInit
makes all properties optional, so that you can pass only relevant data to create()
. To do so, we use a mapped type.
It's awesome that TypeScript's type system is powerful enough to do things like that, but there are limits to the approach. Imagine that we need to change the behavior for specific fields in create()
. For example, to make proto2 required fields required. This isn't possible with a mapped type because the property for a proto2 required field has the same type as the property for other fields.
The solution is to generate an init type for such a message, and to attach it to the descriptor, so that it can be used in create()
. If we export MessageInit
, this solution no longer works, because MessageInit
does not know about the new generated type. We don't have concrete plans to use it at this point, but I hope you understand we'd like to keep this door open if possible.
I also get errors because the
MessageInitShape
is doing too much.
MessageInitShape
does very little. MessageInit
is the complex mapped type (it was called PartialMessage
in v1).
I've pushed up your example in https://github.com/bufbuild/protobuf-es/commit/4f2df5794027aaa506ffaf7d3856929ca929bf60, but I can't reproduce the error. I wonder if your first scenario could be simplified? Messages are no longer classes with v2, and the goal is to make the generated type more versatile:
import type { ProtoGearRatioLimit } from "./gen/ts/extra/issue941_pb.js";
const initialState: Omit<ProtoGearRatioLimit, "$typeName">[] = [
{
gear: 1,
highLimit: 2,
lowLimit: 3,
},
];
This doesn't require any mapped types except for the built-in Omit
. The $typeName
property has upsides and downsides, but it is necessary to solve the problem described in https://github.com/bufbuild/protobuf-es/issues/551 and https://github.com/connectrpc/connect-es/issues/694.
I'm not sure I fully understand your second and third scenario, but it looks like you could straight-up use the generated type here as well? See https://github.com/bufbuild/protobuf-es/commit/dae4d47c4af3e10f97af5e010a994ef39f1c4a0d. Of course you'll need a mechanism to serialize the message for the BFF, but this approach should work well with proto3 messages (I understand that $typeName
can still be a bit awkward).
Do you aim to only use types in the frontend for small bundle sizes?
Thanks for trying to replicate the issue in the tests.
The weird thing is, that in my examples above, I had:
export type IProtoRpcRequest = MessageInitShape<ProtoRpcRequestSchema>;
I copied that right from my file and there was no TypeScript error on that line. I noticed your test had something different so I realized that I should have included the typeof
: MessageInitShape<typeof ProtoRpcRequestSchema>
.
I would have expected TypeScript to complain about using a constant where it is expecting a type. MessageInitShape
without a typeof
generates TypeScript errors in your test project, but doesn't in my project. I don't see anything that is significantly different in that regard. We have the same version of "typescript", 5.5.3.
When I was responding to you I quickly switched back to MessageInitShape
from MessageInit
to see if it still worked and I didn't re-add the typeof
. I knew that at one point, because I mentioned it in the original issue.
When adding typeof
I've resolved all the errors I listed above.
For the second scenario, I initially did call create
in each message creation function and return the fully formed ProtoRpcRequest
, but I realized that then no type information on whether the fields were fully filled out or not would pass through.
The front-end is also directly serializing and de-serializing protobuf messages. I'm trying to add a gRPC service client to the back-end, so then there's the need to transport the resolved messages, so that's how the JSON serialization became a concern.
Anyways, sorry about this chasing of errors. If the exporting of MessageInit
is a no-go, then you can close the issue.
I'm stumped why TypeScript didn't provide a helpful error message 🤷
The generated ProtoRpcRequestSchema
is a value, so typeof
is necessary to use its type as a type parameter.
For the second scenario, I initially did call
create
in each message creation function and return the fully formedProtoRpcRequest
, but I realized that then no type information on whether the fields were fully filled out or not would pass through.
Yeah, in Protobuf, fields are basically always optional. It's an idiom that doesn't always match well with type systems with nullable types. We plan to provide some more options for use cases like this with protovalidate.
If the exporting of
MessageInit
is a no-go, then you can close the issue.
We'll better hold off in it for now 👍
Thanks for opening the issue!
In V2, there is a type,
MessageInit
that excludes$typeName
(and$unknown
) and when passing objects to functions that will callcreate()
it is useful to use a derived type like that.Currently in V2, I have to do something like this:
MessageInitShape<typeof MyTypeSchema>
, but it would be shorter to do this:MessageInit<MyType>
.So this request is to export the type
MessageInit
so it can be imported.Also, having simpler types available might speed up the TypeScript language service, since it seems to have gotten fairly slow while I was trying out the V2 beta.