Fourth oneof option: union-flat? #1077

stevehopkinson · 2024-07-19T19:04:17Z

stevehopkinson
Jul 19, 2024

Hi from one Stephen H to another! I've been using ts-proto extensively for years, so first of all: thanks for the hugely valuable contribution to the community.

Proposal
Add a fourth oneof option, 'unions-flat', which would generate code as follows:

proto:

message Message {
  oneof foo {
    string a = 1;
    string b = 2;
  }
  oneof bar {
    string c = 3;
    string d = 4;
  }
}

type:

type Message = (
  | { a: string }
  | { b: string }
) & (
  | { c: string }
  | { d: string }
)

This appears to combine the advantages of (ease of working with payload, not having to drill down through multiple envelopes, not having to check $case before accessing a key), with the advantages of (constraints preventing multiple exclusive fields being populated, ease of type discrimination). But I've probably missed an issue!

stephenh · 2024-07-20T20:33:10Z

stephenh
Jul 20, 2024
Maintainer

Hello other StephenH! :-)

It took me just a few seconds to "get" the approach, but I get the benefits...

I think historically I've always considered/assumed the foo and bar names of the oneof were "important" and "the real field names" that should "wrap" the a/b/c/d values, but not sure I have a super-strong justification for that.

I'm a little cautious about yet-another-approach to oneofs, but I suppose we're already at 3, so what's a fourth...

Although, really it seems like maybe your proposal is the same as ts-proto's "default" handling of oneoffs (flat fields, of just a/b/c/d), but just with better typing? Is that right?

If so, I'd definitely support, instead of "a new fourth option", just updating our default flat output to have your suggested better typing.

...seems like that would be fine, and I'm sure a breaking change in some way, but it's creating the "same output" in terms of fields/structure, just better types, so I wonder if we could get away with just releasing it as a minor update.

If you'd like to try a PR that implements that, that'd be great!

1 reply

stevehopkinson Jul 22, 2024
Author

'Although, really it seems like maybe your proposal is the same as ts-proto's "default" handling of oneofs (flat fields, of just a/b/c/d), but just with better typing? Is that right?'
Exactly!

'historically I've always considered/assumed the foo and bar names of the oneof were "important" and "the real field names" that should "wrap" the a/b/c/d values, but not sure I have a super-strong justification for that'
That had been my assumption too - but then recently while working with proto libraries in other languages, I found that the default behaviour in those was to 'flatten' the oneof fields as described above. Like you my initial reaction was that these libraries were throwing away 'important' information when they deserialised, as you say - but on further consideration, the name of the oneof 'envelope' field is never meaningful in itself. (Ie. No code should ever run differently if it changes.)

'If you'd like to try a PR that implements that, that'd be great!'
I'm fairly familiar with the code (I had to monkeypatch in handling of 'optional' fields for a proto2 codebase before it was officially supported in proto3 - that was a baptism of fire!) so I'll see if I can carve out the time to give this a look - thanks!

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fourth oneof option: union-flat? #1077

{{title}}

Replies: 1 comment 1 reply

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Select a reply

Fourth oneof option: union-flat? #1077

stevehopkinson Jul 19, 2024

Replies: 1 comment · 1 reply

stephenh Jul 20, 2024 Maintainer

stevehopkinson Jul 22, 2024 Author

stevehopkinson
Jul 19, 2024

Replies: 1 comment 1 reply

stephenh
Jul 20, 2024
Maintainer

stevehopkinson Jul 22, 2024
Author