-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fromPartial
typescript design
#108
Comments
examplecurrent method fromPartial<I extends Exact<DeepPartial<Any>, I>>(object: I): Any {
const message = createBaseAny();
message.typeUrl = object.typeUrl ?? "";
message.value = object.value ?? new Uint8Array();
return message;
} new updated version fromPartial(object: AnyFromPartial): Any {
const message = createBaseAny();
message.typeUrl = object.typeUrl ?? "";
message.value = object.value ?? new Uint8Array();
return message;
} where in this new world, we would have two types instead of one interface AnyFromPartial {
typeUrl?: string;
value?: Uint8Array;
}
interface Any {
typeUrl: string;
value: Uint8Array;
} @webmaster128 do you think there is something like this we can do that's more explicit and simplifies things for the typescript compiler? |
looks typescript is much much happier without Our immediate issue can be solved similar to how this issue which was fixed by this PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For certain protos (google) typescript doesn't like the
Exact
andDeepPartial
(#107)These meta typescript types create an issue that has been talked about here on stackoverflow as well as the ts-proto issues here
These
Exact
andDeepPartial
types were discussed forts-proto
hereThey honestly seem to be adding more issues that what they're providing — to my understanding, you can call
fromPartial
and it allows properties to be optional, and provides defaults.What if we can do something much simpler for the typescript compiler, like explicitly define a typescript interface that has optional properties? Potential drawback for nested properties, but I think it could be worth adding a flag for.
UPDATE: our immediate issue can be solved similar to how this issue which was fixed by this PR
The text was updated successfully, but these errors were encountered: