rescript-labs / decco

Bucklescript PPX which generates JSON (de)serializers for user-defined types
MIT License
226 stars 27 forks source link

Support mutually recursive types #46

Closed jchavarri closed 4 years ago

jchavarri commented 4 years ago

Fixes #45.

The change consists on keeping 1:1 relationship between structure items. So for each structure item with the @decco attribute, another one will be created. If the original structure item had many types in it, the newly created will have multiple value bindings for the encoders / decoders as well.

~This means that there is no need to annotate each type in a mutually recursive type declaration. Just doing~:

[@decco] type inttree =
    | Empty
    | Node(node)
and node = {
    value: int,
    left: inttree,
    right: inttree,
};

~Will generate encoder / decoder for node as well.~

Edit: fake news πŸ˜… Each type annotation still needs to be decorated with @decco:

[@decco] type inttree =
    | Empty
    | Node(node)
[@decco] and node = {
    value: int,
    left: inttree,
    right: inttree,
};

The recursive flag logic got simplified as well, the encoders/decoders structure item gets the same flag as the original type definition. Because recursive is the default, and most of the times the type definition is not really recursive, the warning 39 in generated code has been muted as well.

DCKT commented 4 years ago

I was about to create an issue asking for this feature ! Is this mergeable ? It could be so great !

bnoguchi commented 4 years ago

@jchavarri great work!

@ryb73 thanks for creating and supporting decco! Looking forward to this merge and a new release.

jchavarri commented 4 years ago

@ryb73 I got distracted πŸ˜… I added a couple of tests, could you take a look, please?

jchavarri commented 4 years ago

Not sure why GH didn't pick up the tests result but it passed: https://travis-ci.org/github/reasonml-labs/decco/builds/668041360.

bnoguchi commented 4 years ago

@ryb73 any plans to merge this in and publish a new release to npm soon?

ryb73 commented 4 years ago

Sorry, been really busy and haven't been working on Reason lately. I'm hoping to have time to release this by the weekend.

On Sat, Apr 18, 2020 at 4:24 PM Brian Noguchi notifications@github.com wrote:

@ryb73 https://github.com/ryb73 any plans to merge this in and publish a new release to npm soon?

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/reasonml-labs/decco/pull/46#issuecomment-615940547, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHGK5HRI2HTYTPF2MDQSYTRNIEBBANCNFSM4KBKL5KA .

ryb73 commented 4 years ago

This is awesome, thanks @jchavarri ! Sorry for the delay

bnoguchi commented 4 years ago

Thanks @ryb73 !