Consolidate APIs

[cprussin]

This is a mostly functional WIP investigation into consolidating APIs for different response types (string, binary, stream, etc). With this approach, it's possible to use the response helpers for any body type. This approach also sets up for a fairly obvious path for chunked responses. I'd like to get feedback on it sooner than later, which is why I haven't waited until this PR is totally complete to open it up. Work that remains before I'd be ready to merge this:

• Better docs
• (possible) Should Streamable be put into Node.Stream instead? EDIT: With the changes, there's no longer a Streamable typeclass.
• I'll need to bring testing up to the codebase standards.
• Currently this doesn't play nice with the Lookup typeclass, so things like router { headers } = HTTPure.ok' responseHeaders $ headers !@ "X-Input" yield a type error. I need to figure out what's wrong here. EDIT: This can be solved with a functional dependency on the Lookup typeclass, which I had planned on doing anyways. I'll put the code in this PR to add the functional dependency, and will add a follow up PR eventually to take advantage of the functional dependency to simplify the APIs a bit

EDIT: This PR is now merge-ready!

[cprussin]

Ignore this, was just here for some testing and I forgot to delete it

[cprussin]

This is a Maybe because it won't be present for chunked responses

[cprussin]

@hdgarrood @paf31 I see you guys are the major committers to https://github.com/purescript-node/purescript-node-streams... This module feels like something that belongs there but it's not really a direct mapping to Node APIs... what do you guys think? If you want it, I'm happy to submit a PR to put it there

[hdgarrood]

That’s not the sort of thing I’d want to put in the node bindings - as you say, it doesn’t really correspond to any part of the node api, and also lawless typeclasses are something I’d want to avoid if possible anyway.

[cprussin]

Makes sense, thanks!

[akheron]

Could you briefly describe your idea of how chunked responses would be done with this approach?

[cprussin]

@akheron basically, we'd just add typeclass instances for Stream.Readable, something like this:

instance streamableChunked :: Streamable (Stream.Readable ()) where
  toStream (Chunked stream) = -- Take `stream` and inject chunk sizes between chunks

instance bodyChunked :: Body (Stream.Readable ()) where
  size = pure Maybe.Nothing

Then we just need to modify Response.send to send the Transfer-Encoding: chunked header if Body.size is Maybe.Nothing.

We could also do something more explicit, such as adding a boolean chunked function to the Body typeclass, or changing the size function to return a Effect.Effect String instead so that the instance for Stream.Readable () could implement size = pure "chunked" EDIT: size = pure "chunked" is a bad idea; this doesn't go into Content-Length

[akheron]

I see. Making Body a type class is an excellent idea, but is it necessary to create a stream to write a string or a buffer to the response? How about:

class Body s where
  writeToStream :: Stream.Writable -> s -> Effect Unit
  size :: s -> Maybe Int

This would avoid resorting to JavaScript to create a stream for strings and buffers, as they can be directly written to the response. It wouldn't make it any harder to pump a readable stream to the response. And it would allow request handlers to provide a callback or some other means to generate data on the fly in pure PureScript. Something like this:

-- writeData takes byte count, buffer of data and a boolean flag
-- denoting whether this is the last chunk
HTTPure.response 200 \writeData -> do
  -- get bufSize and buf somehow
  writeData bufSize buf true

[cprussin]

So, I like where you're going with that, but there are two problems that we have to avoid:

1) In order to avoid the problem you mentioned here (namely, it's impossible to type a router function with heterogeneous response types), we must convert values of a type with a Body instance to some common type when constructing the Response value (in other words, the Response type can't contain any type variables). In this PR, I've used a Stream.Readable as the common type, which is convenient as sending the body then is just Stream.pipe. But maybe we can change this to use a partially applied function or something?

2) Since the HTTP.Response object (that is, the underlying Node http response object) is obscured from the router, there can't be any reliance on it until actually applying the Response that the router calculates, that is, when calling Response.send. This restriction could likely be solved with some refactoring and modifications to the API, but I'd bias pretty heavily against revealing the HTTP.Response object to the router (I think HTTPure should avoid ever exposing underlying library objects, except possibly configuration objects for creating the server).

I think I have only a slight preference against using streams the way I've done here if there's an alternative. Since streams are the standard mechanism in Node's http module, eventually, everything becomes a stream anyways. Also, this is a internal implementation detail that consumers won't need to know, unless they choose to implement custom instances. So, my preference here isn't particularly strong.

EDIT: Ignore all of my inane rambling above. Using a partial function works beautifully and solves both of the above problem. Working on wrapping up unit testing in the commit now, will have it in shortly. Getting rid of Streamable entirely, and as of this commit, the Body typeclass and the Response type will then look like this:

class Body b where
  size :: b -> Effect.Effect (Maybe.Maybe Int)
  write :: b -> HTTP.Response -> Aff.Aff Unit

type Response =
  { status :: Status.Status
  , headers :: Headers.Headers
  , writeBody :: HTTP.Response -> Aff.Aff Unit
  , size :: Maybe.Maybe Int
  }

[cprussin]

@akheron updates completed in 0629277, PTAL, thanks!

[akheron]

Beautiful 👍

[paluh]

Hi,

I want to ask about typeclass approach as I'm interested in functional API design in general. This is not a complain in any way because I really appreciate your work on this framework!

I have used purescript hyper and it has the same exact approach (typeclass based API) which for me makes it really hard to use... I've had also used purescript-affjax before it transitioned away from typeclasses and I think that current API which allows explicit selection of a response type is nicer for end user like me. I just don't have to spend time debugging type inference issues related to instance selection any more.

I allow myself to ask some questions regarding type classes usage in httpure. Please ignore them if you don't have spare time or just don't like to answer not very thoughtful questions (for sure I should just read all commits related to these pieces of API before asking about anything).

• I've seen that you have had Body type (data Body = StringBody String | BinaryBody Buffer.Buffer). Was it hard to use such an API based on concrete type (and maybe records of methods)? Was it harder to compose different type of routers then? Did type variables "bubble up" a lot to the public API interfaces?

• How much did you gained using typeclass instead?

[cprussin]

• I've seen that you have had Body type (data Body = StringBody String | BinaryBody Buffer.Buffer). Was it hard to use such an API based on concrete type (and maybe records of methods)? Was it harder to compose different type of routers then? Did type variables "bubble up" a lot to the public API interfaces?

• How much did you gained using typeclass instead?

The biggest drawback with the data constructor approach was that it required a whole new set of helpers for each body type. So, for instance, we have HTTPure.ok as a response helper for a 200 containing some body, but with the data constructor approach, we would need something like HTTPure.okString and HTTPure.okBinary, plus any other body type that we add (and we have plans to add a few other body types, for instance, Stream has already been added). By making Body a typeclass, we are able to use a streamlined set of helpers across the board.

Another big plus is that it makes it a lot easier for end users to define custom body types. You can always define an instance of the Body typeclass for your app, whereas with the data constructor approach, this would be impossible (note that similar results could be accomplished by writing abstractions over the HTTPure helpers in your app, but it's not as straightforward to an end user and is rather brittle). So as a simple example, you could do something like like this in your own app:

newtype JSON = JSON String

instance jsonBody :: Body JSON where
  defaultHeaders = const $ header "Content-Type" "application/json"
  write (JSON body) = write body

...

router :: Request -> ResponseM
router = const $ HTTPure.ok $ JSON "{ ... }"

All this said, note that all the work here is pre-0.8 release and is subject to revisiting. The API is by no means set in stone. I'm more than happy to revisit, and I'm certainly curious to get some more specific examples of exactly why a typeclass would be difficult on an end user? Do you have any to share?

[paluh]

Hi @cprussin,

Sorry for a late response. I've really free (offline) Weekend and really busy Monday.

Regarding problems with typeclass approach I know that because of problems with type inference in our hyper app we ended up with monomorphic type signatures written by hand... we wasn't really able to help it infer types automatically. I think I can provide some code samples, but I'm not sure if they help a lot.
I think that in general with typeclasses user has to be more strict and often provide type singatures even when she/he wants to really prototype something quickly.

I wonder if we could implement dual approach - something like Body.purs (maybe with direct dictionary passing - something like here: paluh@5c4c503) and Body/Classy.purs which uses this base implementation internally.

I'm going to test some other scenarios (maybe Variants based) and provide user experience report if you don't mind ;-)

[paluh]

P.S. Regarding direct record passing - these API can be changed to be more polymorphic like:

response :: forall r. Status.Status -> Body r -> ResponseM

I'm not sure if this could be really useful, but who knows ;-)

[cprussin]

So, I have a few points about this:

1. I'm not convinced that the way in which the typeclass is used here will cause any significant type inference issues in real code. The typeclass instances are defined over reasonably simple monomorphic types and so long as your consumer code has solvable types for the values you pass in to the helper functions--which should always be true for buffers and strings and streams, barring higher level abstractions, for which type signatures are going to be important anyways--I don't really see how this could be a problem. If you can think of any real counterexamples, it would be deeply appreciated and I'd love to see them.

2. I'm also not convinced that we should be eschewing well-supported and standard language features in order to promote folks writing code without type annotations. One of the main goals of HTTPure was to take full advantage of powerful PureScript language features that aren't available in languages like JS, and one of the most powerful such features is typeclasses. I believe this is a textbook use case for a typeclass, so if we were to choose another less powerful language feature here instead, I think we would need a good reason. I don't think it's unreasonable to try to make HTTPure work well with minimal reliance on type annotations in consumer code, but I also don't think it should be a primary design goal that we sacrifice other goals for, since there's a valid expectation that quality PureScript code should have good type annotations.

3. The example you provided using records instead of typeclasses is essentially circumventing the standard language tool in PS to solve this kind of problem in favor of the standard language tool in JS. It appears to me like a more general aversion to typeclasses in general, but typeclasses are such a fundamental part of PS that I'm not sure I see that as a quality argument or good direction for this framework.

So, all in all, I think, given real examples of where the typeclass-based API might be problematic, I'm more than happy to consider alternatives. But without real examples to munch on, I'm not keen on avoiding basic language features that are built to solve exactly the kind of problem that we're using them to solve.

UPDATE: I've done a bit more research around this, looking at sources such as http://www.haskellforall.com/2012/05/scrap-your-type-classes.html. It sounds like the strongest arguments in favor of record passing instead of typeclasses from the Haskell community are based on the way typeclasses worked in H98, and a general aversion to Haskell language extensions. Neither of these arguments are applicable to a purescript world. So, this brings me back to my earlier point: I'd like to see examples of where typeclasses are problems, rather than avoid them simply out of general mistrust.

[paluh]

@cprussin thanks for response!

Maybe I'm overemphasizing the problem here. Of course a lot of arguments in such a discussion is a matter of a personal preference. I don't want to copy arguments from "when to use type classes" google results, but here are responses from many really prominent haskell developers on the topic:
https://www.reddit.com/r/haskell/comments/1j0awq/definitive_guide_on_when_to_use_typeclasses/

1. I'm not convinced that the way in which the typeclass is used here will cause any significant type inference issues in real code. The typeclass instances are defined over reasonably simple monomorphic types (...).

I think that it is a matter of design direction in general. From my perspective if many of such simple interfaces are going to be built using typeclasses just to ease implementation and provide overloading they will finally cause a lot of headaches. It is just dangerous practice to start with from my perspective. If we stick to httpure in our current project maybe I will be able to provide some examples.

2. I'm also not convinced that we should be eschewing well-supported and standard language features in order to promote folks writing code without type annotations (...)

I think that good type inference and possibility to rely on the compiler when programming with "type holes" is a huge benefit of working with PS. Good style is a different matter for me and it shouldn't be forced by an API which complicates inference flow.
I can't argue against "usage of standard language feature", because I also consider type classes as a core and strong PS element. I just prefer to use them when I'm building more strict interfaces or during generic programming or maybe sometimes as a additional replacement for simple value based programming (like in DOM.Classy).

3. It appears to me like a more general aversion to typeclasses in general, but typeclasses are such a fundamental part of PS that I'm not sure I see that as a quality argument or good direction for this framework.

I can assure you that I don't have aversion to type classes in general and I use them a lot in my libraries too... It is necessary tool for any generic programming in PS ;-)

I hope I will be able to provide more concrete examples to this discussion, but like I said I'm afraid that in this small context it will be a matter of style and taste...

[cprussin]

Ok, so how about I propose this: I am skeptical that there is any problem with the direction we're going in, but I'm not outside the realm of being convinced. Since this is a minor sub-1.0 release, the API is subject to change without a major version before 1.0. Let's leave it as-is for now, as it does provide tangible benefits in terms of simpler, more concise APIs. However, I'll open a ticket assigned to you to revisit before 1.0. Hopefully, we can find some more concrete sample code before then and can use it to come to a more well informed decision. Does that sound reasonable?

EDIT: PS I really appreciate you taking the time here to raise your concerns. Thank you!

[paluh]

Ok, so how about I propose this (...) Does that sound reasonable?

Yeah, cool!

Thanks again for your work on httpure!

[cprussin]

@paluh see #115... for some reason, github won't let me assign the ticket to you. In any case, I've added it to the project board for the 1.0 release.

[paluh]

Thanks @cprussin!