elm-export icon indicating copy to clipboard operation
elm-export copied to clipboard

Implements support for algebraic sum types

Open FPtje opened this issue 8 years ago • 9 comments

Please review before merging. Some things were done quickly, and I haven't tested aeson compatibility yet, which I still plan to do.

[update] Aeson compatibility has now been tested. See next post.

See #6

Things done

  • Added tests for encoders and decoders of Timing and Position
  • Implemented encoders and decoders for data types with multiple constructors. It was designed to be compatible with the whims of aeson, though this hasn't been tested just yet.
  • Added a new test type: Monstrosity, which revealed some bugs in the original implementation (and in my implementation of encoders and decoders)
  • Fixed those bugs

Some background

As you mentioned in this issue comment, Aeson is very specific about the json structure it generates. Differences depend on the following factors:

  • Whether one or more constructors are a record type (in Haskell)
  • Whether there are multiple constructors
  • Whether there are constructors that have arguments

Really annoying, but the code in this PR should be compatible with it, though I've yet to test that. I do know, however, that the generated code compiles with Elm 0.18 and is correctly typed. That I did test.

FPtje avatar Feb 26 '17 22:02 FPtje

Alright, I've made a pretty cool Elm <-> ghcjs Aeson conversion test in my elm-marshall-example repository. That repository contains a website running both Elm and ghcjs. When you press a button called test, Elm sends some values of Position, Timing and my Monstrosity through some port, json encoded. Ghcjs listens to the port, prints what it received, and then sends some other values back through some Elm subscription. Elm then shows them in the website.

Here's how it looks: 2017-03-05-133420_1899x1579_scrot

Notes:

  • The website is drawn by Elm. All the console prints are from ghcjs.
  • Never mind the Person stuff in the screenshot. That one is sent back and forth directly without json being involved.
  • Due to some bug in ghcjs, Elm encodes Position to a Javascript value, while encoding Timing and Monstrosity to Json strings. This is not a problem on the Elm side, which can encode to javascript values and strings either way.

FPtje avatar Mar 05 '17 12:03 FPtje

Hi @FPtje, thanks for getting started on this.

I haven't had a chance to review the code in detail, but here are a couple of admin items that will help move this forward:

  • Please change the target branch for this PR to devel (https://help.github.com/articles/changing-the-base-branch-of-a-pull-request/), and rebase the changes onto devel. Hopefully the rebase won't be too painful...

  • The generated code should match the output of elm-format. Also, this is just personal taste but I prefer a case expression to nested if-then-else. So, for example, decodeMonstrosity becomes:

    decodeMonstrosity : Decoder Monstrosity
    decodeMonstrosity =
        field "tag" string
            |> andThen
                (\x ->
                    case x of
                        "NotSpecial" ->
                            decode NotSpecial
    
                        "OkayIGuess" ->
                            decode OkayIGuess
                                |> required "contents" decodeMonstrosity
    
                        "Ridiculous" ->
                            decode Ridiculous
                                |> required "contents" (index 0 int)
                                |> required "contents" (index 1 string)
                                |> required "contents" (index 2 (list decodeMonstrosity))
    
                        _ ->
                            fail "Constructor not matched"
                )
    
    

Open questions, cc @krisajenkins:

  • In this PR, decodeUseless succeeds on any input. How does Aeson encode this type? Something like {"tag": "Useless", "contents": [null]}? Should the decodeUseless then require JSON in this structure?
  • Should the decoders be more strict about the contents field for nullary constructors? For example, this PR encodes the Start constructor as {"tag": "Start", "contents": []}. On the other hand the decoder only inspects the JSON "tag" field, so would succeed on invalid input like {"tag": "Start", "contents": ["Oops"]}. Does Aeson do an explicit check that contents is an empty array for nullary constructors? Do we care?

mattjbray avatar Mar 05 '17 17:03 mattjbray

The branch has been rebased on devel. I've squashed a bunch of commits to make it look a bit nicer. The generated code now matches elm-export, both the encoders and decoders.

To answer your questions:

print $ toJSON () in Haskell gives Array []. Should decodeUseless expect an (empty) array? Maybe, but what would the type of the contents of the array be? Some dummy type? Aeson is pretty strict in the structure. It decodes [] to Just (), but decodes [1,2,3] to Nothing.

That should also answer your second question: Yes, aeson does make sure contents is an empty array. Decoding will fail when it contains stuff or is omitted.

Do we care? Not sure.

FPtje avatar Mar 11 '17 12:03 FPtje

just to chime in, this looks great. i tested against a sum constructor with one and two constructor fields (so something like data Foo = Foo Int Int | Bar String) and it got decoded by a Haskell backend.

charles-cooper avatar Apr 11 '17 20:04 charles-cooper

works great (would be awesome if you could merge this and release another version)

one remark though: AFAIK elm-format is using two empty lines between top-level functions but that's probably a general problem with elm-export

CarstenKoenig avatar Aug 14 '17 17:08 CarstenKoenig

Is anyone actively shepherding this PR or this repo or has it all been orphaned ?

mebassett avatar Nov 07 '17 16:11 mebassett

I've decided against using this in my own projects, since Miso is a really nice Haskell alternative to Elm. I haven't looked at this since.

FPtje avatar Nov 07 '17 16:11 FPtje

Hey when is this going to be merged?

tlentz avatar May 10 '18 18:05 tlentz

I also wonder, what is the status of PR and when it's going to be merged?

chshersh avatar Jul 31 '18 07:07 chshersh