A new Route System? - Rib

I'm loving Rib so far!

But I have found the routing system a little confusing, and it does not really help with the biggest problem in
static site generation (IMHO): broken links.

So I wrote a small mock up of a more type-safe version:

https://github.com/kalhauge/rib-sample/commit/add34d528ed8dd4760c14ba7195be85495cc1ebc

If nothing else, I hope it can start a discussion :)

Cool. I'll take a look. How did you arrive at broken links with Rib.Route?

A Simple example is I can write:

a_ [href_ (Rib.routeUrl (Route_Article "some_missing_post.md")]

Or with css files I often do something like:

a_ [href_ "/my-missing.css" ]

Ideally I would like a rref_ which guaranteed that the route existed.

In your code, you are pre-creating these Route types, and passing it around to the renderer. Isn't that what already happens in rib-sample anyway? i.e., Route_Index :: Route [(Route Pandoc, Pandoc)] takes a list of pre-created routes to individual documents.

The general (undocumented) convention I follow when designing routes is that I would never have to create custom routes (like Route_Article "some_missing_post.md") outside of where the actual file is being processed (eg: forEvery). Once a route is created, I put them in places wherever they are needed (like Route_Index).

But prohibiting the custom creation of routes will adversely affect some non-merely-a-static-site apps like neuron, which does create routes using custom data in places outside of Shake monad. Here's an example: https://github.com/srid/neuron/blob/719fb0bdaf01b8fb2bf0c9de9d3320f124f88e25/src/Neuron/Zettelkasten/Link/View.hs#L41

Regarding the new way of rendering ... IIUC, since your renderer doesn't take a route as an argument, it doesn't seem possible to render parts of the HTML based on the route? For example, how would you do this case block? https://github.com/srid/neuron/blob/719fb0bdaf01b8fb2bf0c9de9d3320f124f88e25/src/Neuron/Web/View.hs#L87-L90

Yes, but the code is made so that you can newer create a Route without also specifying how to generate the text for said route.

makeRoute ::
  Path Rel File
  -- | The relative name of the route
  -> (s -> Action TL.Text)
  -- | The action required to make the route
  -> SiteGen s Route

Yes, but the code is made so that you can newer create a Route without also specifying how to generate the text for said route.

Yes, but like I said in that code you can never create a custom/dynamic route either (which prevents apps like neuron from being possible). For example, how would you do this (from anywhere)? https://github.com/srid/neuron/blob/719fb0bdaf01b8fb2bf0c9de9d3320f124f88e25/src/Neuron/Zettelkasten/Link/View.hs#L41

By the way, renderZettelLink is not used in renderer. (It is being called from the MMark extension runner)

Sorry I cant write this fast :)

I believe that your View could be fixed with a global map with all the routes to all the Zettles, this way it would break if the zettle does not exist.

The second part is the cool part of the structure of the (SiteGen s s) monad.

The s is the Site in my code, and each rendered route have the ability to access the Site and all the available routes in the Site.

I believe that your View could be fixed with a global map with all the routes to all the Zettles, this way it would break if the zettle does not exist.

That's indeed doable, but then the MMark extension would no longer be decoupled from the Site rendering types (and its global map). Right now, it only needs to couple with the route GADT (which is a small type, that only describes the routes).

By the way,

Yes, but the code is made so that you can newer create a Route without also specifying how to generate the text for said route.

If you put Route and the generateSite in Internal.hs (routes can only be created in this module), then wouldn't that solve the problem equally?

Something like this:

renderZettelLink :: forall m. Monad m => Map ZettleID Route ->  LinkTheme -> ZettelStore -> ZettelID -> HtmlT m ()
renderZettelLink routes ltheme store zid = do
  let Zettel {..} = lookupStore zid store
      zurl =  routes Map.! zid

Your Map ZettleID Route would be part of the Site mentioned before, For example:

generateSite :: SiteGen (Map ZettleID Route)  (Map ZettleID Route)
generateSite = do
  Map.fromList <$> forEach ["*.zettle"] \zettle -> do
    let zid = idFromZettlePaht zettle
    r <-  makeRoute zettle $ \(routeMap :: Map ZettleId Route) ->  (... render ...)
    pure (zid, r)

Actually, generateSite and rref in Internal.hs; and not using routeUrl outside of Internal.hs

If you put Route and the generateSite in Internal.hs (routes can only be created in this module), then wouldn't that solve the problem equally?

I want to protect me against me. That is the biggest problem :)

That's indeed doable, but then the MMark extension would no longer be decoupled from the Site rendering types (and its global map). Right now, it only needs to couple with the route GADT (which is a small type, that only describes the routes).

One could argue that the coupling with the GADT is worse, and adding a map from ZettleIDs to a Rib defined Route

renderZettelLink :: forall m. Monad m => Map ZettleID Route -> LinkTheme -> ZettelStore -> ZettelID -> HtmlT m ()

And you'd have to pass Map ZettleID Route all over the project (or use a Reader). I find that to be very inconvenient, not to mention for very little benefit; while Map.! will fail as one would expect (in your design), the current lookupStore already does this anyway (with no need for passing global values around).

One could argue that the coupling with the GADT is worse, and adding a map from ZettleIDs to a Rib defined Route

I don't see that. The route GADT specifies the routes possible. Your Map ZettelID Route includes route type, but now every place where route is created is forced to verify that the route exists, when that could happen only in the renderer (for eg., rref could error out if it is not in the registry or something).

I want to protect me against me. That is the biggest problem :slight_smile:

How would your approach compare, in that regard, against this: putting generateSite and rref in Internal.hs; and not using Rib.routeUrl outside of Internal.hs?

You get to pass routes to renderers (which is not possible in your approach): https://funprog.zulipchat.com/#narrow/stream/218047-Rib/topic/A.20new.20Route.20System.3F/near/194469939

putting generateSite and rref in Internal.hs; and not using Rib.routeUrl outside of Internal.hs?

Oh, actually, that won't work. Nevermind. But renderer problem still remains.

Actually, we can achieve the same without Map ZettelID Route. Just put the route in the Zettel store. But, that level of protection has never been needed in practice in neuron.

This discussion had me thinking about broken links. Correct me if I'm wrong; my conclusion is that - with the rib-sample design, it is impossible to have broken links as long as Rib.routeUrl is invoked only with route passed to renderPage or routes already in the GADT (i.e., no custom route is created outside generateSite). This should go into the rib documentation I think.

Your approach achieves the same goal, however with a significant limitation in the renderer (not being able to render parts of the page based on the route).

You write too fast :)

Regarding the new way of rendering ... IIUC, since your renderer doesn't take a route as an argument, it doesn't seem possible to render parts of the HTML based on the route? For example, how would you do this case block? https://github.com/srid/neuron/blob/719fb0bdaf01b8fb2bf0c9de9d3320f124f88e25/src/Neuron/Web/View.hs#L87-L90

I think that My Idea is that you choose your rendering during the creation phase.

Forexample you could write
renderRouteRedirectHead

You could as well add a boolean to the render to inform it weather to redirect or not.
You could also keep your routes routes as an extra argument to the render.

Or you could change the Page construct I use to have a pageRedirect field or
be a union type?

Neuron is a relatively complex app, but here's a simple one that uses the Route GADT to its full extent. https://github.com/srid/zulip-archive/blob/master/src/Main.hs

I guess if you try to implement your approach in zulip-archive, the value of GADTs might become obvious :-D One thing I'd like to point out is that Rib.Route should always remain an optional feature in rib. But I wonder why you were not able to use Rib.Shake (without copying its fuctions) in your code.

By the way, Rib.Route is based on obelisk-route.

I guess if you try to implement your approach in zulip-archive, the value of GADTs might become obvious :-D One thing I'd like to point out is that Rib.Route should always remain an optional feature in rib.

I think that the main difference we have is my requirement that I want every Route to be a real route,
where you want the Route to encode information. I believe these two requirements might be orthogonal, and might be combined?

But I wonder why you were not able to use Rib.Shake (without copying its fuctions) in your code.

Some of the problems where that I used the custom Monad, the other was that you recently changed out of using the Path library
and I prototyped with it.

We could redesign the Route so that it can contain the GADT route?

data Route a =  Route
  { routeBaseUrl :: Path Rel Dir
  , routeConfig :: a
  }

makeRoute :: IsRoute a => Route a -> (s -> a -> Action Text) -> SiteGen s (Route a)

The "a" in Route a is the value used to render the HTML (or whatever) file for that route. This is why functions (and this includes Rib.routeUrl) which need only the route take Route a as their argument (eg from zulip-archive: routeName, routeCrumbs, etc.), whereas functions that need the value as well would take both Route a and a (eg: renderPage).

Using GADT makes the later kinds of function possible, because the type of a is unified with the specific route passed.

(s -> a -> Action Text)

• I think s ~ a
• This render function still doesn't know the route associated with a. You'd have to explicitly pass it. But what would you pass?

Is there any point in your code where you don't define the route and render the route in the same line?

What do you mean by 'define the route'?

Sridhar Ratnakumar said:

The "a" in Route a is the value used to render the HTML (or whatever) file for that route. This is why functions (and this includes Rib.routeUrl) which need only the route take Route a as their argument (eg from zulip-archive: routeName, routeCrumbs, etc.), whereas functions that need the value as well would take both Route a and a (eg: renderPage).

Uh.. In my example I mean that you would have to define an extra GADTRoute x that instatciate IsRoute.

I was only explaining the Route a from zulip-archive/rib-sample/neuron

Sridhar Ratnakumar said:

What do you mean by 'define the route'?

In your code you create a new Route let r = Route_Article srcPath, right before you render it: writeHtmlRoute r doc:

    Rib.forEvery ["*.md"] $ \srcPath -> do
      let r = Route_Article srcPath
      doc <- Pandoc.parse Pandoc.readMarkdown srcPath
      writeHtmlRoute r doc
      pure (r, doc)
  writeHtmlRoute Route_Index articles

We could refactor this, by defining

writeHtmlRouteArticle doc

and

writeHtmlRouteIndex article

Right; the route is generally created and rendered nearby. And the created route is put back in the a of any other route that uses it (like the index route).

So you store routes (with or without their a) as first class values in the a of other routes (exactly what happens with Route_Index). They are generally not created elsewhere (zulip-archive is an exception)

They are generally not created elsewhere

Except when you do something like Rib.routeUrl Route_Index which is always a safe thing (but Rib.routeUrl $ Route_Article undefined is not; that's always created before, and never created again)

Or factoring out the case analysis:

-- | Define your site HTML here
renderPage :: Route a -> a -> Html ()
renderPage route val = html_ [lang_ "en"] $ do
  head_ $ do
    meta_ [httpEquiv_ "Content-Type", content_ "text/html; charset=utf-8"]
    title_ routeTitle
    style_ [type_ "text/css"] $ C.render pageStyle
  body_ $ do
    div_ [class_ "header"] $
      a_ [href_ "/"] "Back to Home"
    h1_ routeTitle
    case route of
      Route_Index ->
        div_ $ forM_ val $ \(r, src) ->
          li_ [class_ "pages"] $ do
            let meta = getMeta src
            b_ $ a_ [href_ (Rib.routeUrl r)] $ toHtml $ title meta
            renderMarkdown `mapM_` description meta
      Route_Article _ ->
        article_ $
          Pandoc.render val
  where
    routeTitle :: Html ()
    routeTitle = case route of
      Route_Index -> "Rib sample site"
      Route_Article _ -> toHtml $ title $ getMeta val
    renderMarkdown :: Text -> Html ()
    renderMarkdown =
      Pandoc.render . Pandoc.parsePure Pandoc.readMarkdown

Becomes:

 articles <-
    Rib.forEvery ["*.md"] $ \srcPath -> do
      let r = Route_Article srcPath
      doc <- Pandoc.parse Pandoc.readMarkdown srcPath
      writeHtmlRoute ( toHtml $ title $ getMeta doc) $ do
        article_ $ Pandoc.render doc
      pure (r, doc)
  writeHtmlRoute "Rib sample site" $ do
        div_ $ forM_ val $ \(r, src) ->
          li_ [class_ "pages"] $ do
            let meta = getMeta src
            b_ $ a_ [href_ (Rib.routeUrl r)] $ toHtml $ title meta
            renderMarkdown `mapM_` description meta

-- | Define your site HTML here
renderPage : Html () -> Html () -> Html ()
renderPage routeTitle body = html_ [lang_ "en"] $ do
  head_ $ do
    meta_ [httpEquiv_ "Content-Type", content_ "text/html; charset=utf-8"]
    title_ routeTitle
    style_ [type_ "text/css"] $ C.render pageStyle
  body_ $ do
    div_ [class_ "header"] $
      a_ [href_ "/"] "Back to Home"
    h1_ routeTitle
   body


renderMarkdown :: Text -> Html ()
renderMarkdown =
  Pandoc.render . Pandoc.parsePure Pandoc.readMarkdown

That's gonna get unwieldy real fast the more complex your layout becomes. Instead of HTML being defined nicely in one place, it is now spread across!

Not to mention, if you want to render your <head> dependent on route, your renderPage will now take both a custom body and a custom head. And what about breadcrumbs? Then it would take a third HTML.

I can see that :). However, I think that is design decision best left for the individual developer?

Where I think that safe routes would be useful for everybody?

Rib.Route is optional; people don't have to use it. In a way, Rib.Shake too is optional. You can always drop down to using nothing but ribInputDir and ribOutputDir (but still retain the core features of rib).

I think Rib.Route is good enough for static site generation. Your specific requirement of not shooting yourself in the foot (creating of broken links) can, in practice, be solved by the Route a design I explained above. In short, never use Rib.routeUrl with routes other than what's already in the a.

In short, never use Rib.routeUrl with routes other than what's already in the a.

Well this will only work if you only put valid routes in the a.
I, however, rest my case :)

Have you ever had that happen (put invalid routes) to you when using the rib-sample approach, though? You can create invalid routes like below, but it seems quite unlikely for a bug like this to come up.

    Rib.forEvery ["*.md"] $ \srcPath -> do
      let r = Route_Article srcPath
      doc <- Pandoc.parse Pandoc.readMarkdown srcPath
      writeHtmlRoute r doc
      let invalidR = Route_Article "I don't exist"
      pure (invalidR, doc)
  writeHtmlRoute Route_Index articles

I mainly had problems with the static files, and with index files, like Route_Archive, or Route_Index, which
I sometimes accidentally delete.

I can't stop thinking about it. Is there a reason why the GADT Route and the data have to be seperate, If you always
create the route and render the data in the same location:

Why not join the data and the route?
In this case I'll have another datatype called LocalUrl (which resemble my kind of Route).

data LocalUrl = ...

data Route =
  Route_Index  [(LocalUrl, Pandoc)]
  Route_Article  FilePath  Pandoc

Now we can get away from the higher order type constructs.
This is almost the same code:

-- | Define your site HTML here
renderPage :: Route -> Html ()
renderPage route  = html_ [lang_ "en"] $ do
  head_ $ do
    meta_ [httpEquiv_ "Content-Type", content_ "text/html; charset=utf-8"]
    title_ routeTitle
    style_ [type_ "text/css"] $ C.render pageStyle
  body_ $ do
    div_ [class_ "header"] $
      a_ [href_ "/"] "Back to Home"
    h1_ routeTitle
    case route of
      Route_Index val ->
        div_ $ forM_ val $ \(r, src) ->
          li_ [class_ "pages"] $ do
            let meta = getMeta src
            b_ $ a_ [href_ (Rib.routeUrl r)] $ toHtml $ title meta
            renderMarkdown `mapM_` description meta
      Route_Article _ val ->
        article_ $
          Pandoc.render val
  where
    routeTitle :: Html ()
    routeTitle = case route of
      Route_Index  _-> "Rib sample site"
      Route_Article _ val -> toHtml $ title $ getMeta val
    renderMarkdown :: Text -> Html ()
    renderMarkdown =
      Pandoc.render . Pandoc.parsePure Pandoc.readMarkdown

I think the thing I'm trying to say Is that it seams like Route, in its current form, is more a data structure to
help rendering over helping to route.

For example the extra type variable tells us nothing about the route except for what data that route
took to be created. We can of cause add special new types wrappers around that data to make it origin
clear. That Route Index != Route Archive.

Anyhow, I really appricate you work on this project, and I hope that I have not wasted your time on this Idea.
I really just like the Idea of having safe-by-construction routing :). I can see that the added complexity might not be worth it.

What's the definition of LocalUrl?

I just though I would call my kind of Route LocalUrl instead so we did not confuse the two concepts:

data LocalUrl = LocalUrl
  { urlBase :: FilePath
  , urlRest :: FilePath
  }

Your Route type is not a route; it is more than a route. It would be appropriate to call it Page. Indeed , that's what rib used, and consequently route urls were just string (and not as type safe). https://github.com/srid/rib-sample/pull/9/files#diff-f8f3412da88cd4806f23d59fe59ebc3bL32-L36

So for all intents and purposes it is a String. Your type really is then:

data Page =
  Page_Index  [(FilePath, Pandoc)]
  Page_Article  FilePath  Pandoc

You can't use Rib.routeUrl here obviously.

were just string (and not as type safe)

I think that is my point, both of these approaches are not "type-safe", Strings because you can just create them, and GADT Routes because you can just create them.

The difference is that the GADT Routes are guaranteed to be formatted correctly, which increases the chance that it points to something, but does not gurantee it.

So for all intents and purposes it is a String. Your type really is then:
You can't use Rib.routeUrl here obviously.

No, It's not just a string. It's a guarantee that the item in the end of the string exist :)

Where would be that guarantee if I invoke renderPage $ Page_Article "/does-not-exist" somePandocDoc?

The Idea is to hide the implementation of LocalUrl from the user.
The user can only get a LocalUrl from the makeLocalUrl (makeRoute) mentioned from before.

In this case the filepath of Page_Article "/does-not-exist" somePandocDoc is never used
is only used to encode the route, so I expect it to be something like:

lurl <- makeLocalUrl (chageExtension filepath ".html" ) $ \s -> renderPage (Page_article somePandocDoc)

Like I said before, try implementing your approach in zulip-archive or neuron, and then I guess what I'm saying will become obvious. Show me a full real-world project that uses your types, and I can be convinced.

It will have to satisfy the following:

• Be able to render in one place, with individual parts of the HTML rendered depending on the routes (including sub routes).
  • Imagine being somewhere deep inside the DOM tree, and using case'ing on routes to render differently (this can happen either in renderPage or in some other function it uses)
• Allow creating values that say "this is the route to this page" (without necessarily requiring the data used to render it; for example I want to be able to create a route value for a particular zettel given only its ID, but without also requiring the Markdown document data), so as to be able to pass it around. This route value should be type-safe in that it can't be arbitrary string, but it has to point to the individual pages, so that the above case'ing can be done.

I think I will be able to do a mock-up in zulip-archive without changing too much of the code.
I'll see when I get time to hack on code again :).

Thank you again, for taking the time to discuss this with me!

Actually neuron may be a better example, as zulip-archive uses it idiosyncratically

This route value should be type-safe in that it can't be arbitrary string, but it has to point to the individual pages, so that the above case'ing can be done.

Out of curiosity, Do you ever do case analysis on a route you are referencing?

Yes, checkout the neuron source code. For example, routeOpenGraph function (but also the Web/View.hs module).

Am I wrong or are routeOpenGraph only run on the "rendered" route and not on any references?

References? It is used only during rendering.

But a route value is not necessarily used only by rendering.

My point is that you only use the a part of the Route store graph a when rendering that exact route,
newer when you in one route refer to another.

Anyway it's getting late for me. Nice chatting with you.

Hi again!

I think I might have made my case poorly yesterday, however, I spend the morning implementing the new
route system in neuron. It was surprisingly easy. Extending your code were a breeze, much praise to you from here :).

I have made two commits, to show that the two approaches are orthogonal:

The first commit adds safe LocalUrl, with builtin guarantees that they exist, without changing the
routing system.

https://github.com/kalhauge/neuron/commit/640d8e3f235110e7a5a781dc829380bf0b640e6c

The second commit notices that a Route is only used in rendering that route, and is therefore always present with it's data. This means that we can embed the data in the route:

https://github.com/kalhauge/neuron/commit/326113735953502dfbc699b8e137f8baeb36fb89

Both of these commits compile, but I have not tested them with a real website. I hope that this answers both your
bullet points. 1) I have not changed the rendering, 2) Use the safe LocalUrl to refer to other pages. 2b) You newer
really do caseing on references to other pages in Neuron, so I was not able to test that out. Most of the time I recon
that you know what you are referencing.

I don't know if the safe LocalUrl makes sense in the context of Neuron because it the internal routing is pretty simple.

But after moving the LocalUrl to its own module Rib.Site , I think at least it would make a good module in Rib, for sites
with more complex routes?

@Christian Kalhauge I looked at your diffs. That's an improvement (to your original proposal), however you are still passing the site value (that represents the entirety of the static site) all over the code base. This is really bad. Why does a markdown extension, for example, need to know about the entire static site layout? This doesn't improve on Rib.Route; it is worse (and I don't buy the broken links argument). And the second diff also made the Route type ... not a route at all.

But after moving the LocalUrl to its own module Rib.Site , I think at least it would make a good module in Rib, for sites with more complex routes?

Do you have a demonstration of your Rib.site being more suitable than Rib.Route for more complex routes (those diffs to neuron do not demonstrate it; they only "complect" things, as Rich Hickey would say)? In any case, you can just make this a library, and put it in hackage under the name say rib-site.

Let me address your concerns in order:

However you are still passing the site value (that represents the entirety of the static site) all over the code base. This is really bad. Why does a markdown extension, for example, need to know about the entire static site layout?

I think it is the difference between explicit and implicit coupling. The markdown extension will need to have access to the Routes of the system,
and can create all of them. The Site variable is an explicit reference to all routes of the system. We can, however, improve coupling and only
allow the markdown to access the part of the site we really need:

class HasZettelLookup s where
  lookupZettel :: MonadFail m =>  s -> ZettelID -> m LocalUrl

renderMarkdown :: HasZettelLookup s => s -> ZettleID ->  ... -> Html ()
renderMarkdown s zid ... =
   ...
  url <- lookupZettel s zid
   a_ [Rib.rref_ url ]
   ...

This markdown render makes it clear that it depends on a site that have a ZettelID lookup,
this can then be implemented by the site. So, total decoupling.

And the second diff also made the Route type ... not a route at all.

I think this is my point. When you use the route as a route, and not as a page you never use the complex types,
so instead you can use a simpler type as LocalUrl when you use them as routes, if you accept the cost of passing around the site.

Do you have a demonstration of your Rib.site being more suitable than Rib.Route for more complex routes.

I hope that the more complex routes could improve things like,

1) Routes that depend on the data. For example, it is common practice to add the hash of static files like JS
and CSS files to the end of the filenames to improve Caching. For example style.css -> style-abef92031.css.
This would not be possible with a route system like yours.

2) Creating a SiteMap in the system is easier, because we know everything that can possibly be routed to.
I think this is an issue in https://github.com/srid/rib/issues/104.

I really enjoyed this exchange of Ideas but I get the feeling that I will not convince you :). I might just implement rib-site in the future on my own, but do let me know if you change your mind. In that case I would love to help you implement it in Rib.

Thanks again and keep the good work up on Rib.

1) It would not be impossible.
2) A sitemap is basically just a list of routes. Your Shake action, which generates these routes, can accumulate them (just as the article routes are accumulated for Route_Index in rib-sample) and generate the sitemap at the end.