Comments (21)
I just did a small test of the gRPC generation in a test project. It works well, and it is really easy to get going. I don't think I have played with it enough to come with any sort of constructive feedback, but first impression is great.
What I plan to do is to make a branch in one of my bigger projects and migrate to this version, and then replace my own gRPC code with this, and then I might have some more thoughts.
from protoc-gen-elm.
What's your use case?
For one piece of response data, I just want to fall back to some sane defaults if it can't be transformed according to a set of criteria. I figured the generated default functions provided those sane defaults for my use case. I'm sure there are other use cases.
from protoc-gen-elm.
[email protected] should generate re-exported "default..." functions
from protoc-gen-elm.
I just released 3.0.0 so I'm going to close this.
from protoc-gen-elm.
Your idea of using files as a replacement of (otherwise not available) namespaces for types is pretty cool.
Let's take this a bit further.
Protoc complains if there any top level messages that have a name collision.
So instead of generating a file Proto.Messages
for a file messages.proto
, we could instead generate a file per message, i.e. Proto.OrderStatus
. Nested types could then go into a nested directory - Proto.OrderStatus.Processing
.
I would tend to not add a new module for oneofs and instead use "_" (just like currently) to make name collisions unlikely.
What do you think?
from protoc-gen-elm.
I think underscores are fine if the goal is to keep the module tree flat. Although very long identifiers might be annoying to refer to in code.
The structure I had in mind with generating more modules was something like: keep the "one Elm module per protofile" structure we have today, and in addition generate a submodule for each message type that has nested data types. So if OrderStatus
is in messages.proto
, generate a Proto.Messages
module where OrderStatus lives, and also generate a Proto.Messages.OrderStatus
module in which Processing etc. live. Thinking about it, it seems that can still cause name clashes with protobuf packages. I wonder how Prost handles that. Let me check that out.
I'm not sure what's best, honestly.
from protoc-gen-elm.
I used some Prost and Tonic recently and quite like the way they handle the scoping. I'll try to implement equivalent code generation for Elm:
- Generate one file per package
- Generate one file for each message with nested types inside it
- Use Underscores to namespace enums
For your example (assuming package order
, protoc-gen-elm would generate:
- A file
Proto/Order.elm
:- A type alias OrderStatus with a field status:
type alias OrderStatus = { status: Maybe OrderStatus_Status }
- A custom type OrderStatus_Status:
type OrderStatus_Status = OrderStatus_Processing Proto.Order.OrderStatus.Processing | OrderStatus_Succeeded Proto.Order.OrderStatus.Succeeded | OrderStatus_Failed Proto.Order.OrderStatus.Failed
- A type alias OrderStatus with a field status:
- A file
Proto/Order/OrderStatus.elm
- A type alias for each the inner messages:
Processing
,Succeeded
andFailed
.
- A type alias for each the inner messages:
from protoc-gen-elm.
I ran into a tricky recursive modules problem:
In protobuf, the following file is completely fine:
message Message {
bool idk = 1;
}
message Oneof {
oneof msg {
Message opt1 = 1;
Message opt2 = 2;
}
}
but in prost's generated code (and what I wanted to do as well),
this generates two modules: A parent module, and a child module for the OneOf message.
The child module then defines a union type and en/decoders for the oneof msg
part.
This seems fine, but since a type from the outer scope is used, we have to import the parent module from the child module, which results in a recursive import.
In this particular case, the issue can be solved by just using a type alias, but once a union type comes into play, the types are no longer structurally equivalent and we cannot make the code compile without importing.
So it seems like we can only solve this by using handcrafted namespacing via "_" seperators in the parent module and then export more readable aliased versions in the child module. We could also use some kind of .Internal
module to prevent the ugly names from making the IDE experience worse.
from protoc-gen-elm.
Right! That's an interesting case.
Could this be solved by not generating a new module for oneof fields? Or in general, not for fields at all. In other words, only generate a new module for messages that have other messages or enums declared within them?
So in the above case, we'd still end up with only one module, as there is no nested message in there, and the en/decoders for oneof msgs
would also reside in this one module...
Actually, no that still wouldn't work in the case where a nested message refers to an outer message in the same package. I guess this is fine in Prost, because cyclic dependency graphs of modules are not illegal in Rust. But the same is not true of Elm.
The internal modules approach could be interesting to explore (and would be even better if Elm allowed us to expose a module to only a particular set of modules without wrapping it all in a library, or if you could have local libraries). Do you have a particular approach in mind? I'm guessing you mean having a module where the generator just dumps all type definitions, namespaced with underscores, and then expose type aliases that refer to them in the more structured modules?
from protoc-gen-elm.
Yeah exactly, I'll just dump everything in one file. So far this approach seems to work out. The code is extremely ugly but since it's generated and not the "public API" anyways, it should be fine.
from protoc-gen-elm.
Hmm while messages work fine, enums are problematic with this approach.
If we for example define an enum
package Test;
enum E {
A = 0;
B = 1;
}
we can generate code in the internals module like
type Proto__Test__E = Proto__Test__E_A | Proto__Test__E_B
but we lose the ability to pattern match when we define an alias.
Do you have an idea how we could solve this?
from protoc-gen-elm.
That's a shame. And it's not only enums, then, but also oneof variants. Honestly, I can't think of a good solution off the top of my head. Maybe it isn't meant to be...
Maybe the best thing we can hope for really is to use prefixes as namespaces within a package. Perhaps the best we can do to avoid name clashes is my first suggestion in the OP:
Prefix each variant with the custom type name as well (type OrderStatus_Status = OrderStatus_Status_Failed OrderStatus_Failed | ...)
If nothing else it's a first step, and as far as I can see, a workable - if not quite ideal - solution to the name clash problem.
from protoc-gen-elm.
I'll try experimenting with remapping the union types via a generated function.
In that case, as a user you will have to remember to call the generated conversion functions but the code should still look nicer:
Instead of
import Proto.PackageName exposing (MsgName_OneofName(..))
case oneOf of
MsgName_OneofName_OptionA _ -> ...
MsgName_OneofName_OptionB _ -> ...
import Proto.PackageName.OneofName exposing (convertOneOfName, OneOfName(..))
case convertOneOfName oneOf of
OptionA _ -> ...
OptionB _ -> ...
What do you think? Can you think of good names for the converters in the two directions?
The generated documentation for the types should probably mention the two generated functions.
from protoc-gen-elm.
I have published [email protected] which implements a more advanced module system which is package/message scope based instead of file based. You can try it out and see if it fixes the issue for you.
I'll need to add some tests and documentation, but so far it looks pretty good.
Side note if you are using tonic as well: grpc generation should work as well with the env variable EXPERIMENTAL_GRPC=true
from protoc-gen-elm.
Alright, thank you. I'll check it out and give you some feedback over the weekend.
from protoc-gen-elm.
I didn't know this had gRPC support. I've just rolled my own implementation of the gRPC-Web protocol in Elm so far, using this library as support for encoding/decoding messages.
from protoc-gen-elm.
I did a small amount of testing, and have some thoughts.
-
I discovered another edge case resulting in name clashes. Consider the following message:
message TestRequest { message Inner { message Inner {} } oneof inner { string s = 1; uint32 n = 2; } }
The type generated by the oneof (TestRequest.Inner.Inner) competes with the type generated by the inner inner message (also TestRequest.Inner.Inner). I thought some sort of a prefixing scheme for oneofs might help, but it seems like there would always be the possibility of a name clash, and it would make the resulting API worse, which is what we were trying to avoid in the first place. That said, I suppose one way to look at it is that we want to eliminate the vast majority of name clash cases for the majority of reasonably sane protobuf code, rather than to guarantee no name clashes 100% of the time.
-
The inconsistencies in how oneofs are generated compared to inner messages irks me a bit. I realise this is the original solution we were discussing in this thread, but it does feel a bit strange, doesn't it? That oneofs always generate a new module, whereas inner messages don't necessarily. I wonder if this is a case where we should just say "good enough", and get on with our lives.
I suppose, number 1 here is also technically a manifestation of the original problem in this thread, in a way, although certainly more obscure. It only happens when you have a message inside a message inside a message where the two innermost have the same name, and you have a oneof in the outermost message with the same name as the other two messages. How often does that happen? (famous last words)
All that said, I really like how the recent changes have lead to a nicer API. It looks so much cleaner, and I think it addresses the most common of name clash cases. Thank you for doing this!
from protoc-gen-elm.
Regarding the new name clash case, I just noticed prost/tonic has a similar issue, although there it's a problem even without the crazy nesting. I'd say it's not worth it to spend too much time on this.
from protoc-gen-elm.
Yeah, it's pretty much impossible to avoid all name clashes and generate a reasonable module graph. I think it's fine though, as you said, the standard use cases are clash-free and you kind of have to try hard to break it.
As long as there isn't a good use case for stuff like doubly nesting messages with the same name as a oneof or using the same name with different casing (innerMsg vs InnerMsg vs inner_msg...), I'm fine with what we have right now.
Regarding 2) I would tend to live with the inconsistency.
Did you try the Grpc generation? I put it behind a flag because I wanted to experiment with the API with my own app. If you have thoughts on how to improve it feel free to open a new issue :)
I love the feedback :D
from protoc-gen-elm.
Hey. gRPC works well. I ported my entire project and haven't run into any problems.
One comment unrelated to gRPC: I'm missing the functions to get default values for messages. They are only in internal now. It would be nice to have them available outside of internal, as well.
from protoc-gen-elm.
Hey. gRPC works well. I ported my entire project and haven't run into any problems.
That's great to hear!
One comment unrelated to gRPC: I'm missing the functions to get default values for messages. They are only in internal now. It would be nice to have them available outside of internal, as well.
Ah interesting, I didn't need the default values so far, so I didn't bother to re-export them.
What's your use case?
from protoc-gen-elm.
Related Issues (11)
- All integer types encode/decode as int32 HOT 5
- Support for Int64 Datatypes HOT 2
- Odd generation of optional fields HOT 8
- proto comments are not transferred in the generated files HOT 1
- Indistinguishable messages HOT 7
- explicitly specifying plugin path HOT 1
- Dev tools support HOT 8
- Get rid of fromInternal/toInternal functions in favor of generics HOT 3
- Code repetition while encoding nested messages HOT 1
- Importing Enums generates Code that fails to compile HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from protoc-gen-elm.