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

Add support for recursively defined message structures. #26

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

EdSchouten
Copy link
Contributor

I'm trying to use this library in combination with a relatively complex
protocol, namely the remote execution protocol of the Bazel build sytem:

https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/remote_execution.proto

Some of the dependencies of this protocol contain recursively defined
messages. These currently cause the compiler to emit Elm code that isn't
valid, due to recursive alias types.

Solve this by adding a "%sMessage" custom type for every message. This
causes the recursion to be broken. Let all nested messages use this type
and fix up the encode/decode functions to properly destruct/pack
messages into the custom type.

I'm trying to use this library in combination with a relatively complex
protocol, namely the remote execution protocol of the Bazel build sytem:

https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/remote_execution.proto

Some of the dependencies of this protocol contain recursively defined
messages. These currently cause the compiler to emit Elm code that isn't
valid, due to recursive alias types.

Solve this by adding a "%sMessage" custom type for every message. This
causes the recursion to be broken. Let all nested messages use this type
and fix up the encode/decode functions to properly destruct/pack
messages into the custom type.
@EdSchouten EdSchouten force-pushed the recursive-types branch 2 times, most recently from 5bb3db3 to 7302c47 Compare April 9, 2019 08:17
@EdSchouten
Copy link
Contributor Author

@EdSchouten
Copy link
Contributor Author

(Friendly ping?) :-)

@tiziano88
Copy link
Owner

Thanks for the PR! I was wondering, would it make sense to just avoid the type alias, and define a new (inlined) type for each message?

e.g.

current:

type alias Bar =
    { field : Bool -- 1
    }
type BarMessage = BarMessage Bar

proposed:

type Bar = Bar
    { field : Bool -- 1
    }

Then we only use the new type everywhere, even though it requires always explicitly referring to the constructor (both for creating new instances, and also for pattern matching).

WDYT?

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 this pull request may close these issues.

2 participants