Skip to content
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

Protobuf oneOf to generate coproducts without Option-ality #113

Closed
SemanticBeeng opened this issue Jul 15, 2019 · 3 comments · Fixed by #119
Closed

Protobuf oneOf to generate coproducts without Option-ality #113

SemanticBeeng opened this issue Jul 15, 2019 · 3 comments · Fixed by #119

Comments

@SemanticBeeng
Copy link

SemanticBeeng commented Jul 15, 2019

In proto3 there is no required constraint available.

As a consequence this

oneof value {
  A : a = 1;
  B : B = 2;
}

generates

Option[A] :+: Option[B] :+: CNil

Proposing the Option-ality is removed to generate shapeless A :+: B :+: CNil instead.

Rationale:

  1. The Option-ality seems redundant in the coproduct for evident reasons: if A is present in the coproduct then it is not optional.
  2. Generates more complex than necessary service apis.
  3. Slows down the runtime marshalling.

Given that skeuomorph is schema transformation library the user should have more flexibility at the time use from mu (time of service api design) and not be bound by the semantics of source IDL (context #91).

Resources

  1. https://stackoverflow.com/questions/42955621/require-a-oneof-in-protobuf
  2. https://news.ycombinator.com/item?id=18188519
@SemanticBeeng SemanticBeeng changed the title Protobuf oneOf to generate coproducts and coproducts of Option Protobuf oneOf to generate coproducts and not coproducts of Option Jul 15, 2019
@SemanticBeeng SemanticBeeng changed the title Protobuf oneOf to generate coproducts and not coproducts of Option Protobuf oneOf to generate coproducts without Option-ality Jul 15, 2019
@AdrianRaFo
Copy link

AdrianRaFo commented Jul 19, 2019

The right protobuff syntax would be

oneof value {
  A a = 1;
  B B = 2;
}

and AFAIK skeuomorph will generate the Coproduct without Options. Am I wrong @rafaparadela?

@rafaparadela
Copy link
Member

I agree that optional values in Coproducts is redundant and doesn't make sense.

We might want to keep this:

case ProtobufF.TNamedType(name) => TOption(A.algebra(TNamedType(name)))

but maybe we should just optimize the printer for Coproducts

@rafaparadela
Copy link
Member

In the case that @pepegar and @BeniVF approve this approach, this is the PR that closes this issue:
#119

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants