Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: platform agnosticism #44

Open
schell opened this issue Sep 19, 2017 · 12 comments
Open

Question: platform agnosticism #44

schell opened this issue Sep 19, 2017 · 12 comments

Comments

@schell
Copy link
Contributor

schell commented Sep 19, 2017

How difficult would it be to make this library "platform" agnostic? This would mean removing any reference to jsaddle, reflex-dom and ghcjs-dom. Doing this would make it a shoe-in for reflex-sdl2.

@imalsogreg
Copy link
Owner

There's a newly servant-client-core package which tries to abstract the Client type family away from a concrete http client.

So, if we use that library, we can get the Client type family for free, with the consequence that client types servant-reflex provides would look much more like Int -> Bool -> IO Text than Dynamic Int -> Dynamic Bool -> Event () -> IO (Event Text).

We can drop reflex-dom and instead use reflex and ghcjs-dom, if we port servant-reflex to ghcjs-dom's XmlHttpRequest machinery. That would take a little work.

I think jsaddle is ok on any platform, including sdl2?

Paging @3noch :)

@schell
Copy link
Contributor Author

schell commented Sep 19, 2017

My thinking is that if we abstract away what does the actual async request, we could get away without including jsaddle and *-dom. Of course, it's then a bit less useful ... or ... convenient.

I'm thinking something like this:

type AsyncRequestRunner a = GenericRequestThing -> m (Maybe a)
-- Or maybe we include async and make this `GenericRequestThing -> m (Async (Maybe a))`

clientWith :: (Reflex t, MonadIO m, PerformEvent t m) => AsyncRequestRunner a -> Dynamic t Int -> Dynamic t Bool -> Event t () -> m (Event a)

Lots of hand waving.

@schell
Copy link
Contributor Author

schell commented Sep 19, 2017

Yeah, I don't know if there's an easy way to do this for all reflex-* libraries. reflex-dom has the advantage (or disadvantage) that it's pretty much single-threaded, but in reflex-sdl2 these events have to come back to the main thread if they are to drive GL updates.

@3noch
Copy link
Collaborator

3noch commented Sep 19, 2017

@schell I'm sure we could come up with some sort of common "request" and "make request" abstraction that could be filled in by different reflex hosts.

@imalsogreg
Copy link
Owner

This PR against servant looks very promising! https://github.com/haskell-servant/servant/pull/818/files

@roberth
Copy link

roberth commented Mar 23, 2018

Although it may not be as feature complete at this point, I've created a platform agnostic rewrite. It's sufficient for my own use now, but feel free to create issues (or PRs ;) ). If you think it'd be better to merge it into this project that's also fine with me (not sure if a good idea though, considering that this is about removing dependencies)

@mightybyte
Copy link
Collaborator

Would it be possible to get out of the business of Events and Dynamics altogether and have a pure function that simply converts Servant routes into XhrRequests?

@roberth
Copy link

roberth commented Mar 23, 2018

@mightybyte It would be possible, but not very pretty. With a custom RunClient function, you could retrieve the Request. You can convert that to an XhrRequest (except you wouldn't if you're avoiding reflex-dom, which this issue is about). But don't you want to use servant for the response?

What is your need that is not served by servant-client-core, reflex-servant or servant-reflex? reflex-servant is already out of the business of Dynamics.

@mightybyte
Copy link
Collaborator

@roberth Oh, that's a good point about the Request. I was more trying to think of ways to ruthlessly simplify things, but I had forgotten about the Request component.

@roberth
Copy link

roberth commented Mar 26, 2018

@mightybyte Apart from the uncurrying it's already a trivial wrapper around servant-client-core.

@imalsogreg
Copy link
Owner

@roberth Awesome work! I'd been struggling with converting to a tuple-based API, and fitting anything Event-based into servant-client-core seemed impossible given the client monad typeclass they provide. I like your solutions in both areas. It would be great to have your work as a PR in servant-reflex, where we could explore further the cost/benefit of hosting multiple API options in a single package. But if you prefer to keep your version isolated, simple and with minimal dependencies, that's understandable too.

@roberth
Copy link

roberth commented Mar 27, 2018

@imalsogreg Although I appreciate the idea of a PR wrt visibility and collaboration, at this point I don't see a technical benefit of doing so. I don't see any code that I can reuse or the other way around. After reflex-servant issue 2 the API for options may become similar though, except reflex-servant doesn't depend on reflex-dom types for the 'request fixup' function. That's handled by servant-client-jsaddle.

It's also entirely possible to use both packages side by side. I've done it while migrating my app and I could have stopped half-way without any problems.

Feel free to copy the code into servant-reflex for experimentation or release. Also, I'd be happy to merge any changes into reflex-servant or to reconsider your proposal when new insights arise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants