Routing

[cryogenian]

No description provided.

[jdegoes]

Review by @garyb / @paf31. Will review myself as well.

[garyb]

Version fields in bower.json are redundant at the moment as it only uses git tags to resolve released versions anyway.

[cryogenian]

bower warns if version in bower.json and tag are different.

[jdegoes]

But if you delete it, then it won't warn at all.

[cryogenian]

removed version

[garyb]

Sorry if it seems I'm being picky about the naming of things :) I just wasn't sure what they meant without having to dig through the code a bit.

[paf31]

Extract function dropHash = replace rgx ""?

[paf31]

I'll make specific comments on the individual lines, but what I'll say here is more general. This is going to seem a bit ranty, for which I apologize, but I've been planning to write something like this up for a while.

Edit: this has turned into a DSL tutorial, sorry. Take it for what it is - I think this would be a nice way to think about the problem.

This library seems like a really good candidate for a "layers of abstraction" approach. I'll explain what I mean by that. The pieces are all there, but I think there may be a better way to arrange them.

• Generally, the goal is going to be to extract a pure component from your library, and a thin impure component, which is basically going to amount to calling hashChanged.
• We can go further, and split the pure component into layers, each building on the last one, adding specific use cases - layers of DSLs of increasing levels of specificity, with type class laws providing the logical glue between layers.
• The layers are modules, and module exports (including instances) define how layers interact at the boundaries, so choosing module exports is important.

In detail:

• At the bottom, we have an AST Route for routes. Routes are made up of Path components and Parameters. You already have this in a slightly different form, and it looks fine, but you might want to rename a few things, or zip keys and values into Tuples. Type classes give us general ways of constructing Routes:
  • Semigroup: Path is a semigroup, and so is Route, where we glue paths end-to-end, and concatenate parameters.
• We need to be able to parse routes from strings. purescript-parsing is a nice option here, but you might be able to implement something with regexes or just String.split.
• We need a type of errors which will be raised when a match fails. We want:
  • Semigroup: to combine multiple errors for a single match
  • Semiring: to combine errors "in parallel" - when we have two different matching functions, and they both fail.
  • We can choose a very simple implementation of errors: choose some base error type E and then form type MatchError = [[E]].
• Next, we need a way to match routes. What is the denotation of such a matching function? On the level of types, it is Route -> Maybe a, where a is some useful data structure that we want to get out, but we want more structure. Again, what type classes do we want?
  • Functor: we should be able to map over the as
  • Apply: we should be able to pair up two consecutive matches
  • Applicative: we want trivial matches
  • Alt: we want to be able to try two alternative routes and take the first one which succeeds
  • Alternative: we want to be able to define filters which always fail
  • Bind or Monad? I would argue that we don't need these. Monad allows us to define matching functions which branch depending on a component of the route, but we can describe any finite amount of branching with Alt, and routes are finite, so the extra expressiveness isn't useful. Also, applicative matching functions can be statically analysed, but monadic matching functions cannot. You can't automatically derive documentation for monadic routes for example.
  • We need some "atoms" in our language which we can glue together with the instances above:
    • lit :: String -> Match Unit - match a literal string
    • var :: Match String - bind a variable
    • param :: String -> Match String - match a specific key
    • anyParam :: Match (Tuple String String) - match any key/value pair
• We can use classes to define this without choosing a representation. This is the "final" representation:

class (Alternative f) <= Match f where
  lit :: String -> f Unit
  var :: f String
  param :: String -> f String
  anyParam :: f (Tuple String String)

We can already start writing matching functions without choosing a representation. Laws tell us what these routes mean:

data FooBar = Foo String | Bar Number Boolean

route :: forall f. (Match f) => f FooBar
route = Foo <$> (lit "foo" *> var)
    <|> Bar <$> (lit "bar" *> (num <$> var)) <*> (bool <$> param "baz")

Aside: if you want to be able to generate routes from values as well as parse them, then one nice thing about this approach is that it works in different categories. I think you can just switch Functor for Invariant, and similiar things for Apply, Alt. So then for every parser, you get a pretty printer too. You probably have to be a bit careful about things like num and bool though.

• Now we need to provide a model for our theory. There are many models: we can choose a model which creates Markdown documentation, or even one which works in reverse and synthesizes example routes with placeholders for variables and parameters, but the one we need is going to match routes. So choose a very naive representation to begin with, like

newtype MatchImpl a = MatchImpl (Route -> Either MatchError a)

We want to be able to write instance Match MatchImpl. We can use instances at the lower level to define the instances we need here: Semigroup gives us Applicative and Semiring gives us Alternative.

Then we can write runMatch :: forall a. (forall f. (Match f) => f a) -> Route -> Either MatchError a. This is the function which we expose from our module.

• The final layer of abstraction is just cosmetic: we can write operators, type classes and functions which allow us to construct route matching functions in more idiomatic ways:

route = ("foo" </> var) <|> ("bar" </> var <?> "baz")

This is probably the least important layer from the point of view of the theory, but quite important when it comes to using the library. The important thing is that we're building a language on top of an existing theory, which provides laws to guide us.

Other comments:

• Names are going to be important because there are some concepts which overlap a bit. A Route is a parser in your docs, for example, but there is also the concept of the "current route".
• Documentation would help a lot. It's hard to guess what the types represent in some cases.

[paf31]

Do you need to use decodeURIComponent or something similar here?

[cryogenian]

added

[jdegoes]

Awesome feedback, @paf31. Maxim, think you can make these changes or would you like some assistance / guidance from @paf31?

[cryogenian]

• renamed some function, hope their names are better now
• remove unneeded fields from package.json
• added decodeURIComponent
• added monoid to Router

[cryogenian]

@paf31

• purescript-parsing handles not only template parsing but error composition too.
• AST of router is linear, so, there is no need to handle error parallel.
• ParserT already has all this behaviours (Alt, Alternative, Functor, etc)
• Signals from hashchange can be invariant with application state. But I believe that they not have to. Especially in cases of handling subroutes, where one can want (this one is me, though) not to emit messages of parent state.
• Adding types to those messages (like bool or num) can be made by adding specific instances for RouteDiff now. I suggest that adding specific parser for this case will work also.

[paf31]

purescript-parsing handles not only template parsing but error composition too.

Yes, you're probably right, especially if we stay with the parse-from-template approach. I'm just thinking out loud more than anything with this one.

AST of router is linear, so, there is no need to handle error parallel.

In a particular template, yes, but what if you want to handle two different types of routes? A Semiring of errors will handle distributing the sequential errors over the alternatives so that you'll end up with something like

(Expected "/foo") + (Expected "/bar" * RequiredParam "baz")

which you would then be able to postprocess to a more meaningful error message like:

Expected one of "/foo" or "/bar"

[cryogenian]

@jdegoes
I hope I can make this changes myself. Or maybe with little help of @paf31. And I'll make them if you want.

But, personally, I don't see the point:

• Lib can handle routes
• Lib can handle subroutes
• Lib emits typesafe messages (more or less, though, if one don't provide RouteDiff instances there will be only Tuple String (StrMap String)
• Lib handles setting of specific states as hash.

[cryogenian]

@paf31
Why do you need this messages? I think it's not very difficult to make them in my representation by adding field to data Router = Router {error :: String, parser :: ParserT ...}

[paf31]

Yeah, as I say, just understand my notes as an alternative approach, and to a certain extent, me trying to understand the problem. If everything works as needed, maybe don't worry about it. I put the more important items (decode, etc.) that I saw in line comments.

[cryogenian]

@paf31 Thank you very much for review! It's very helpful.

[jdegoes]

But, personally, I don't see the point:

It's a free lesson in how to organize a library from the creator of PureScript. 😃

I think @paf31's suggestions would improve the organization of the library even if they wouldn't add functionality. If you agree, then go ahead and make them, if you don't agree, well we can discuss further on Skype or just merge the PR as-is.

[cryogenian]

Great! I'll try to try @paf31 approach in different branch and make pr in day or two