eskimor / servant-purescript

Translate servant API to purescript code, with the help of purescript-bridge.
BSD 3-Clause "New" or "Revised" License
105 stars 44 forks source link

QueryParams should be wrapped up in Maybe #30

Closed felixSchl closed 7 years ago

felixSchl commented 7 years ago

QueryParams are currently optional on the server side. There is discussion to lift this requirement: https://github.com/haskell-servant/servant/issues/241, but until this happens I think we should wrap up query parameters in Maybe to mirror the server side.

eskimor commented 7 years ago

I cant' really remember my reasoning back then, I think I had one :-P . Anyway, if this poses some problem, I'd gladly accept PRs!

eskimor commented 7 years ago

Would you say this is a bug? Sorry for the really long delay, I somehow missed this issue.

mpodlasin commented 7 years ago

Hi. Is anyone looking into this? Lack of optional parameters causes me a lot of pain - my endpoints can be filtered in many different, almost always optional, ways.

Are there any known workarounds for this one? I tried to wrap optional parameters in yet another Maybe, but purescript serializes Nothing to null, while Haskell expects the string to be Nothing.

eskimor commented 7 years ago

I don't believe someone is. A simple workaround that could work, is specifying your route twice once with queries and once without. Obviously this does not scale if you have more than one or two query parameters.

It should not be too hard to generate a Maybe a for query parameters, you could give it a try. If you have questions or get stuck, feel free to ask in a comment to this issue.

mpodlasin commented 7 years ago

Ok. I will dig into it!

mpodlasin commented 7 years ago

Hm... But do we want to make wrapping query parameters in Maybe an option, or do we simply do breaking change?

eskimor commented 7 years ago

I don't think query parameters used a lot right now, so a breaking change would be the cleaner approach. Also because it is usually what one wants - I believe.