search

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

Elm bridge #49

Closed k-bx closed 5 years ago

k-bx commented 5 years ago

Continuation of https://github.com/haskell-servant/servant-elm/pull/43

In addition to migrating to elm-bridge, this PR also migrates to Elm 0.19. Things left in this PR:

Out of this PR scope:

dawehner commented 5 years ago

This PR is a really nice step towards a more useable library.

check all the elm-sources and make sure they type-check (~5 were checked)

A couple of examples are still targeting elm 0.18. When I try to update them I still run into

-- NAMING ERROR ------------------------------------------ elm/Generated/Api.elm

I cannot find a `NoContent` type:

132| getStatus204 : Http.Request NoContent
                                 ^^^^^^^^^
These names seem close though:

    Dict.Dict
    Set.Set
    Bool
    Dict

Hint: Read <https://elm-lang.org/0.19.0/imports> to see how `import`
declarations work in Elm.

Here is a pull request which updates at least the examples to 0.19 as well

think if maybeBoolToIntStr can be done in a better way than putting immediately after imports

To be honest I think that's better than putting it after the generated code.

mkUrl properly implemented (Dynamic/Static stuff)

Do you want to elaborate on that point?

cache is basically always nuked, think if it's ok

The library seems absolutely fast enough, so that seems fine.

think on the NoContent situation better (I failed to resolve it nicely)

How about the following? Instead of generating it's own custom type NoContent how about using() everywhere as type, so for example the request could be of type Http.Request ().

check on some real-world codebase

Sorry I haven't done that either yet. I first need to update to elm http 2.0 to be honest :|

check that examples in README and in examples folder are on par with what code generates (I only fixed tests)

I hope I've done that in the PR: https://github.com/k-bx/servant-elm/pull/1

k-bx commented 5 years ago

Fixed rest of elm-sources, replaced NoContent with () (for now at least)

k-bx commented 5 years ago

think if maybeBoolToIntStr can be done in a better way than putting immediately after imports

To be honest I think that's better than putting it after the generated code.

The question here is that it would probably be nicer to do something like putting it in an Elm library, like elm-format, instead of unwrapping in every generated module.

mkUrl properly implemented (Dynamic/Static stuff)

Do you want to elaborate on that point?

Yeah, what I did was a rather quick and dirty hack, didn't think through the whole mechanics. Basically just commented the old logic. Here's the current code:

mkUrl :: ElmOptions -> [F.Segment EType] -> Doc                                                                                                                                                                                                                                                                              
mkUrl opts segments =                                                                                                                                                                                                                                                                                                        
  "Url.Builder.absolute" <$>                                                                                                                                                                                                                                                                                                 
  (indent i . elmList)                                                                                                                                                                                                                                                                                                       
    ( map segmentToDoc segments)                                                                                                                                                                                                                                                                                             
    -- ( case urlPrefix opts of                                                                                                                                                                                                                                                                                              
    --     Dynamic -> "urlBase"                                                                                                                                                                                                                                                                                              
    --     Static url -> dquotes (stext url)                                                                                                                                                                                                                                                                                 
    --   : map segmentToDoc segments)
k-bx commented 5 years ago

Good news! I've not only checked this on real-world codebase (added few fixes), but also ported to http 2.0.0 as I went https://github.com/k-bx/servant-elm/pull/3

k-bx commented 5 years ago

0.6.0.0 is out! Please check it out everyone http://hackage.haskell.org/package/servant-elm

dawehner commented 5 years ago

Nice! I'm looking forward to using it

On Tue, 28 May 2019, 14:32 Kostiantyn Rybnikov, notifications@github.com wrote:

0.6.0.0 is out! Please check it out everyone http://hackage.haskell.org/package/servant-elm

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haskell-servant/servant-elm/pull/49?email_source=notifications&email_token=AAAHH3RSYZHWY2P4F4MB553PXUX6NA5CNFSM4HDLAZU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWMEFWA#issuecomment-496517848, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAHH3SKUKBPLRHXJH4W5K3PXUX6NANCNFSM4HDLAZUQ .

domenkozar commented 5 years ago

Great work, will attempt to port our codebase from all the forks to use 0.6.0.0 and see how it goes :)