Indistinguishable messages about protoc-gen-elm HOT 7 CLOSED

Hi!

Yes, that is expected behaviour. Using extensible records for structural instead of nominal typing allows for generic methods for multiple data types (i.e. you can write a method that takes any record which has a "users" field of type List String) and
less typing.

Generating nominal typing is pretty difficult. Because of the Elm module system not featuring nested modules or recursive imports, while the proto module system has nested modules, we would need to generate multiple nominal types and convert between them. In fact, this is what we already do with enums and recursive messages.

I do agree though, that the structural typing can have some disadvantages, for example, if we have multiple data types with the same fields but different protobuf field numbers. Then it's actually really important that the correct encoder/decoder is used, but the type system does not catch it.

But to be honest, after I added gRPC support, I haven't run into this issue at all - in this scenario, the correct encoder/decoder call is automatically generated anyways.

What do you think?

from protoc-gen-elm.

Maybe Request/Response analogy isn't correct in this context. What I mean is more like:

syntax = "proto3";

message Request {
  oneof request {
    RequestA request_a = 1;
    RequestB request_b = 2;
  }
}

message RequestA {
  repeated string users = 1;
}

message RequestB {
  repeated string users = 1;
}

I can use different messages that have the same structure and I need to know which message exactly I'm getting. I learned about oneof and it seems to solve the issue for me but the Elm code that handles it is quite lengthy. Am I using in correctly? Is there a way to make it shorter maybe?

-- encoding
{ users = [ "user x", "user y" ] }
    |> Proto.Request.Request.RequestA
    |> Proto.Request.Request.toInternalRequest
    |> (\request -> { request = Just request })
    |> Proto.encodeRequest
    |> Protobuf.Encode.encode

-- decoding
data
    |> Protobuf.Decode.decode Proto.decodeRequest
    |> Maybe.andThen .request
    |> Maybe.map Proto.Request.Request.fromInternalRequest

from protoc-gen-elm.

Ah yes, oneof should be the correct choice in this szenario.

I'm afraid this is more or less the code you will have to write. I'd suggest importing with an alias, like Proto.Request.Request as Request, and removing the lambda in favor of inlining the request field, but in general the code looks fine.
I'll try to explain why the different parts of the code are necessary:

|> Proto.Request.Request.RequestA

This is an obvious one, we need to choose which option of the oneof we want to use.
The module name is generated this way because people often do stuff like

message OneOf {
  message OptA {}
  message OptB {}
  oneof msg {
    OptA optA = 1;
    OptB optB = 2;
  }
}

in which case we need OptA and OptB as type aliases as well as type constructors.
Therefore, OptA/OptB is generated in Proto.OneOf and the union type would be generated in Proto.OneOf.Msg.

|> Proto.Request.Request.toInternalRequest

This one is unfortunate and a result of the mismatch between protobuf scopes and elm modules.
I've been thinking if we should offer alternate constructor functions like Proto.Request.Request.requestA which are defined simply as Proto.Request.Request.RequestA >> Proto.Request.Request.toInternalRequest.

|> (\request -> { request = Just request })

This is necessary since a protobuf message can hold more than one oneof/more than one field.
oneofs are always optional in protobuf, that's why the Maybe wrapper is there.

|> Proto.encodeRequest
|> Protobuf.Encode.encode

There is a difference between the Protobuf Encoders and actually running them on a type in that in the Encoder form, they can be composed easily. For example, encodeRequest internally uses encodeRequestA and encodeRequestB.

This would be another one where we could potentially generate helper functions to remove some boilerplate at the cost of a larger API surface. For example, we could generate the current encoder as requestEncoder and generate encodeRequest as the composition of that and Protobuf.Encode.encode. My fear is that this could be even more confusing.

The decode steps follow pretty much the same logic/explanation.

I hope this helps, let me know if you have any more questions or feedback.

from protoc-gen-elm.

@andreasewering Thanks a lot for explanation and insights.

I would be happy if I could replace these lines:

Proto.Request.Request.RequestA { users = [ "user x", "user y" ] }
    |> Proto.Request.Request.toInternalRequest
    |> (\request -> { request = Just request })

with just:

Proto.Request.Request.requestA { users =  [ "user x", "user y" ] }

I think that (\request -> { request = Just request }) could also be a part of the alternate constructor. If I create some data, then I assume I won't pass Nothing there. So I guess the alternate constructor:

requestA data = 
  data
    |> Proto.Request.Request.RequestA
    |> Proto.Request.Request.toInternalRequest
    |> (\request -> { request = Just request })

would be a great addition. But I don't have broader context - perhaps there is a reason to not have it.

Please, let me know what you think.

from protoc-gen-elm.

But what if you decide to expand your message like this for example:

message Request {
  oneof request {
    RequestA request_a = 1;
    RequestB request_b = 2;
  }
  // new!
  string another_field = 3;
}

Then your function does not work anymore, since Request is now a record containing 2 fields.
So the most simplification we could do with an alternate constructor would be going from this

{ request = Proto.Request.Request.RequestA data |> Proto.Request.Request.toInternalRequest  |> Just
, anotherField = "something"
}

to this

{ request = Proto.Request.Request.requestA data
, anotherField = "something"
}

Which isn't bad I suppose. Feel free to make a PR if you need this feature, otherwise I'll probably implement it in the next few weeks.

from protoc-gen-elm.

Got it. Thanks for explanation.

I created a draft PR to add constructors #222.

from protoc-gen-elm.

Thank you for your PR and sorry for the late response.
Merged and included in version 3.2.0.

from protoc-gen-elm.