haskell-servant / servant-elm

Automatically derive Elm functions to query servant webservices
BSD 3-Clause "New" or "Revised" License
164 stars 48 forks source link

When query param is a bool, Elm code attempts to use String.fromInt on it #54

Closed szg251 closed 4 years ago

szg251 commented 5 years ago

Problem description

I ran into a problem with version 6.0.2.

One of my endpoints accept a boolean parameter:

"todo" :> QueryParam "done" Bool :> Get '[JSON] [Todo]

The generated Elm code looks like this:

getTodo : Maybe Bool -> (Result Http.Error (List Todo) -> msg) -> Cmd msg
getTodo query_done toMsg =
    let
        params =
            List.filterMap identity
                (List.concat
                    [ [ query_done
                            |> Maybe.map (String.fromInt >> Url.Builder.string "done")
                      ]
                    ]
                )
    in
    Http.request { ... }

The problem is with the String.fromInt function. The query_done is a Maybe Bool, but it tries to stringfy it with the wrong funciton.

Solution

After looking at the code, my guess is there is no distinction of types in the toStringSrc function. (Elm 0.18 used to have a toString function that stringifies any type but in 19, we need different functions for different types).

https://github.com/haskell-servant/servant-elm/blob/master/src/Servant/Elm/Internal/Generate.hs#L400

szg251 commented 5 years ago

Later I will try to solve this and create a PR.

k-bx commented 5 years ago

Yes! The way I solved this in my code was a shameless "find and replace" I have for a bunch of parameters (unique by their name). But a better solution is definitely welcome here!

This open-source codebase doesn't have a replacement specifically for this problem, but a general mechanism can be seen https://gitlab.com/k-bx/meetup/blob/master/backend/meetup/src/Meetup/GenerateElm.hs#L156

szg251 commented 5 years ago

I still didn't have time to work on it, but in my fork of an earlier version (before migrating to elm-bridge) I had this kind of solution:

https://github.com/gege251/servant-elm/blob/master/src/Servant/Elm/Internal/Generate.hs#L471-L487

szg251 commented 4 years ago

I've been working on a solution here: https://github.com/haskell-servant/servant-elm/pull/59

Every instance of toString is replaced with a function, that checks the type. My only concern is the maybeBoolToIntStr function: I removed this function with the new polymorphic one, but if I use the same approach in query params, using "1" as True and "0" as False, I get a parse error on the server side. So I used "true" and "false" instead, but I am not sure, if it would break the original implementation.

k-bx commented 4 years ago

I think it should be fine now. If someone is certain it's not -- please open a new issue or reopen this one.