protobuf-ts icon indicating copy to clipboard operation
protobuf-ts copied to clipboard

Update oneof representation for v3

Open jcready opened this issue 3 years ago • 2 comments

See https://github.com/timostamm/protobuf-ts/issues/143

jcready avatar Apr 03 '22 18:04 jcready

Apologies for the delay, I'm preoccupied with personal matters at the moment, but will get back to this ASAP.

timostamm avatar Apr 12 '22 12:04 timostamm

@timostamm I think with my latest commit that the PartialMessage type should work correctly now. I hope you're okay with the updated "unset" oneof group type: { kind: undefined; value?: never } which helps limit when we need to escape a field named kind.

I'm still confused about the deprecation comments.

Also, please confirm you'd like me to change the merge base for this PR to 3.x. See screen shot. Alternatively I can just open a separate PR against 3.x. image

jcready avatar May 07 '22 21:05 jcready

We are planing to use this package in one of our projects. It seems that this PR would solve the problem that strictNullChecks are required for oneofKind as described here which we unfortunately can't activate in our code base.

How long do you think it is going to take to release this change in a new version?

krisztianb avatar Nov 08 '22 13:11 krisztianb

@krisztianb I have made a few utility types and functions which should allow you to effectively convert between the the two oneof representations (i.e. the existing oneof style and the one proposed in this PR) both at the type level and during runtime. Obviously it would be easier if the PR landed, but at least with these utility functions/type you should be able to get a lot of the benefits of the v3 oneof representation (just with a bit more work).

The way I wrote the utility functions/types still depends on strictNullChecks but I believe you can easily modify them to work without stringNullChecks by changing the AnyOneof type to:

export type AnyOneof = { oneofKind?: string; [k: string]: unknown };

And changing the Oneof type to:

export type Oneof<
    T extends AnyUsefulOneof,
    K extends AnyUsefulOneofKind<T> = AnyUsefulOneofKind<T>,
    M extends AnyUsefulOneofValueMap<T, K> = AnyUsefulOneofValueMap<T, K>
> = M[keyof M] | { oneofKind: undefined };

Here's a TS Playground for you to play around with (it should have strictNullChecks disabled).

jcready avatar Nov 08 '22 14:11 jcready

Thanks for your help @jcready. We are currently looking into ts-proto which seems to be more actively maintained which is an important criterion for us when choosing a library.

krisztianb avatar Nov 12 '22 10:11 krisztianb

@krisztianb FWIW ts-proto has the same (oneof=unions) or worse (default) oneof representation as what currently exists in protobuf-ts v2. If you're looking for an actively maintained protobuf code generator/runtime for typescript which uses the the oneof representation outlined in this PR you should check out protobuf-es.

Also, as far as the updated oneof representation for v3 goes, it will eliminate TS errors when accessing the value field of a oneof group, but that value will still be optional/nullable w/o strictNullChecks.

jcready avatar Nov 12 '22 16:11 jcready

@jcready the difference with ts-proto concerning the "oneof" union representation is that it does not add a | { type: undefined } "member" to the discriminated union type which is what is causing problems for us when not using strictNullChecks.

As to protobuf-es: this seems to be a rather new library (version number beginning with 0 indicating it being not well tested) for ES without type definitions for TS. Will have to look into it further.

krisztianb avatar Nov 13 '22 10:11 krisztianb