Mithril.js: [rewrite:router] RouteResolver signature, redux

[pygy]

Passing vnodes as arguments to the RouteResolver methods is arbitrary if not harmful.

The vnode is onmatch is artificial (in that it isn't expected to be ever passed to render/render), and sometimes even invalid (the tag can be the RouteResolver).

In render, the common use will be to pass the vnode argument as (part of) the return value, but doing the same in a view is a big no no...

Building on #1276, I'd suggest

• onmatch(resolve(payload), args, requestedPath) (since m.route.get() would still return the last route at this point)
• render(payload, args)

And have the default render build a vnode out of its parameters.

[pygy]

Possible drawback for render(payload, args) is that, for RouteResolvers without an onmatch method, the first payload argument doesn't make any sense...

Am I the only one who cares about this?

[barneycarroll]

I care about it deeply. I believe high level thinking behind this API makes it impossible to justify — what works works because it works.

[pygy]

The onmatch / render core functionality makes sense to me... unless we introduce an optional, more "ecologic" kind of component (one that doesn't take down its whole tree when it leaves), then render is moot. Lumberjack components are a good default, though.

onmatch(resolve, args, requestedPath) <= fine for me

But for render, there's no good solution that I can think of. Beside the points raised in my first two messages, render(args, payload) has the optional payload last, but then the payload, which will usually be a component when present becomes the second argument after args/attrs, the m() order is reversed. render(_, args) all over the place is bad.

render( { payload, args } ), maybe? How would we call the argument?

{
  render: function(foo) {
    return m(Layout, m(foo.payload, foo.args))
  }
}

Not very good either in that usually the payload _will_ be a component and we impose the payload term.

[gilbert]

I think we can do away with foo.payload considering it's not difficult to do async require without mithril's involvement: https://github.com/lhorie/mithril.js/issues/1095#issuecomment-242093355

[pygy]

@mindeavor but passing a payload makes it trivial to implement a global layout...

[gilbert]

But layouts are easily done via a function... I don't think it's worth adding complexity for.

[pygy]

Implementation-wise, assuming #1290 lands, it is trivial to change this line to read

var render = typeof route.render === "function" ? route.render : defaultRender

Could you give an example of a "layout done via a function"?

[gilbert]

The code itself is simple, but the mental overhead of using it (and teaching it) is not. I'm suggesting that passing payload (also known an vnode) to render is unnecessary, and should be removed for the sake of router usage simplicity.

I think the problem is we're thinking in terms of components rather than view functions. If the function was the focal point of the router, I think we could get a nice api. I'll try to get a working example.

Edit: Here it is, and it works! https://github.com/lhorie/mithril.js/pull/1296

[pygy]

That doesn't simplify things conceptually.

The return value becomes the resolution signal, and the match time/render time distinction is pushed to userland conditionals.

As @barneycarroll commented, that proposal mixes concerns kind of like config/if(init) did in v0.x...

How is it easier to teach than the current onmatch/resolve/render system?

@barneycarroll in #1296: It does raise the issue of why deferred resolution is important in the current implementation, which arguably only provides a new hook in onmatch as a way to say the previous route shouldn't teardown until we have our new component. Bear in mind my major criticism of the current API is that it arbitrarily separates and constrains features over too large a surface without offering the granularity you'd expect, rather than saying such functionality isn't desirable in the first place. If we think it's OK to simply perform logic in the view to determine whether resources are loaded or not, we can do this all by using the state object (or in your API, by mutating route) [snip]

The nice thing with the current API is that it mimics the way the browser works. When you click a link, the current page remains active until the request succeeds or times out.

[barneycarroll]

@pygy I appreciate the value of deferring resolution, I just think the current mechanism in practice combines the worst of both worlds: we have the UI dead time of "web 1.0" while we wait for a new UI to be fetched, and _then_ when it resolves we can deal with the interstitial wait for populating data that "web 2.0" made famous. It does offer a generic and relatively pain free solution for doing what it does, but I think it's inflexible and underpowered compared to what we have in v0.2.

[pygy]

Now I start to understand what you find limiting.

You can always load modules in the background using setTimeout or in reaction to onmouseover/ontouchstart...

function preload(module) {
  return function(e) {
    e.redraw = false
    require([module]) // webpack async loader
  }
}

//...
m('a[href="/foo"]', {oncreate: m.route.link, onmouseover: preload("foo.js")}, "foo")

Do you have other scenarios in mind?

[barneycarroll]

It's essentially what you're saying here. If you can persist state (or ctrl as we used to call it) - which has never been a problem before - and we can say hold off drawing - which again is a solved problem - then the author can deal with all of this stuff as their application architecture and UX preferences dictate. I think my route components API is mostly there, except for redraw strategy, which I need to figure out a more solid API for. I'm really happy to see this proposal develop though.

[gilbert]

@pygy It's easier to teach because it's a single function that has one primary responsibility: return the view. If you decide you don't want to show a view, you have some easy-to-understand tools to construct the behavior you want (reject, m.route.set(), route.retry()).

[pygy]

@barneycarroll https://gist.github.com/barneycarroll/9de48c5fb7763873b974d9ebcca10e88 gives a 404 error..

Why would we want to hold off drawing? Having the ability to keep the old view live while the next module is loading seems like a good thing. Otherwise we're back to startComputation().

@mindeavor I think I'm begining to understand where you're coming from, but I still disagree. The only thing your proposed system simplifies is the onmatch => render optional data flow through resolve(). The other concepts are still there:

• your function works as an augmented render
• reject only works when onmatch would have fired, returning it at a later point (by mistake) will crash the diff engine
• m.route.retry() is resolve() in disguise.

The onmatch vs render distinction becomes implicit, and the code in the anonymous function must be written in a peculiar style to accommodate these assumptions.

The current API may have a slightly steeper learning curve, and may be constraining as @barneycarroll pointed out, but it is explicit and once you understand it, it is hard to misuse.

Explicitness and robustness trump other concerns for me.

[gilbert]

@pygy Right, I'm not suggesting my proposal has different features than the current router. The thing my proposal simplifies, as you said, is the onmatch => resolve => render optional data flow. You say "only thing", but I say "big thing" – it's a big deal to me that render no longer receives a payload that is strangely different whether or not you're using onmatch.

Secondly, removing async from the router will remove the need and complexity of a "limbo" state that exists in-between routes. Instead, a mithril app is always in a defined route; to get to another route, you must first meet all the requirements. This is a more robust approach to program, IMO.

You have a great point about a user potentially returning reject later as a mistake. I will update the code to throw a proper error message in such a case.

[pygy]

So you're basically objecting to the logic on this line which, I agree is a bit arbitrary and more convoluted than I would like too.

The idea is that, as long as there's no render function, Mithril builds a vnode out of the route parameters and either the Component end point or the value passed to resolve(). If you render() manually, then building the vnode is up to you, and the slot populated by resolve may default to "div", depending on what happens (or doesn't happen) upstream in onmatch().

Do you expect people to have problems understanding the above?

Using a vnode to present the payload (which may not always be a component) in render feels weird to me for the reasons explained above, but the flow is not that complex.

In #1290, the as far as the user code is concerned the state (both the current view and the result of m.route.get()) always corresponds to the last fully resolved route, even if another route has been requested. window.location changes as soon as the request is made, though, which is close to but not exactly how browsers work when following normal links. AFAICT after poking at Firefox, the location doesn't change until the response starts coming, then the URL changes and the old page remains visible and scrollable (but not (fully?) reactive) while the new one loads.

Is it the discrepancy between route.get() and window.location, or the inability to cancel a pending route that you consider a limbo?

[barneycarroll]

The idea is that, as long as there's no render function, Mithril builds a vnode out of the route parameters and either the Component end point or the value passed to resolve().

...

Do you expect people to have problems understanding the above?

It is conditional complication and certainly not intuitive. It takes a while to get at a minimum and the tradeoffs of the different possible configurations (What happens if I want to render and resolve? What's more important to me: asynch resource loading the canonical way - or view diffing) aren't easy to rationalise. The reason me and @mindeavor are sinking time and work into prising these issues apart and figuring out alternative APIs must vouch for that to some degree.

Using a vnode to present the payload (which may not always be a component) in render feels weird to me for the reasons explained above, but the flow is not that complex.

I keep coming back to this (maybe if I do it enough I'll hit upon a description that gels): keeping stateful reference between disconnected lifecycle methods via the stateful argument that's passed to every single lifecycle methods is not weird at all: it is and always has been the default. Without grokking that, you can't build an app. Appealing to someone for whom that's too complicated... with a convenience API for code-splitting... is like getting on the motorbike before you can walk. Even the most trivial of Mithril apps will need to hit state (v1) or ctrl (v0) in order to store arbitrary values and attrs (v1) or further arguments (v0) in order to receive passed in data. It's facetious to say this is too much for a Mithril user to grasp in comparison to the RouteResolver's current payload API.

[pygy]

@barneycarroll Yeah, both onmatch() and render() make sense on their own, but their interplay is a bit clunky.

keeping stateful reference between disconnected lifecycle methods via the stateful argument that's passed to every single lifecycle methods is not weird at all

The RouteResolver object is used as context for both onmatch and render and could be used to that end, but it comes with the usual ES5 ergonomics issues of this scoping.

You'd want to keep a per RouteResolver state hash? Created before onmatch fires an persisted across render(). How would you call it? I don't like route (#1296) much because we already have m.route, using that name again makes it harder to describe and discuss the router without ambiguities.

[gilbert]

@barneycarroll Are you suggesting an API like this?

m.route(document.getElementById('app'), '/', {
  '/users/:id': {
    onmatch: (route, resolve) => {
      route.user = User.fetch( route.args.id )
      route.user.run(resolve)
    },
    render: (route) => {
      return m(Home, { user: route.user })
    }
  },
  '/signin': SignIn
})

If so, I like it much better than the current router API.

[pygy]

It's good when the render() hook is present. How would it work in its absence? Defining a field on route could work. (using route too since it is the best proposal so far, but I'm still not enthusiastic)

component is too restrictive, since you can also pass strings (which could make sense for a custom element).

Maybe tag like the vnode field? payload? Probably too generic...

If anything, that's an advantage of the current system, the resolve argument doesn't need an official name.

{
  path: "/foo",
  args: {bar: "baz"},
  tag: {view(){}} // user-provided when `render` is absent
  foo: bar // arbitrary, user-provided fields to be used in `render`...
}

[gilbert]

I still think it's better for mithril's router not to go out of its way to support code splitting. The API should be intuitive for single-file SPAs first, and then maybe support code splitting after that.

I view onmatch without render as a convenience edge-case. If we were to just make onmatch coupled to render, you could code-split this way:

m.route(document.getElementById('app'), '/', {
  '/': {
    onmatch: (route, resolve) => {
      require(['Home'], Home => {
        route.page = Home
        resolve()
      })
    },
    render: (route) => {
      return m(route.page, { user: route.user })
    }
  }
})

The flow is explicit, easy to understand, and easy to abstract.

Even then, I think it's better to have an express.js-router-like API. It's a tried and true solution; why not take a good idea and run with it?

[pygy]

onmatch isn't just about code splitting. You can run arbitrary logic and resolve() synchronously. Think #1180, #1298, auth, and maybe other scenarios.

Both hooks have their own set of use cases, we need to find a way to make them work nicely both on their own and together.

The [handler(req, res, next),...] system is interesting for the onmatch phase (that's what Express handles), but it is not compatible with the need for a fresh vdom tree on every redraw().

[gilbert]

I know onmatch is not just for code splitting, but my main beef with the API is the unpleasant interaction between onmatch and render via the vnode param, which AFAICT is only justified by code-splitting.

On the topic of an mithril-express router, it turns out it _is_ compatible 😄 Check it out:

m.route(document.getElementById('app'), '/', {
  '/': Home,
  '/users/:id': [
    function fetchUser (route, next) {
      route.args.user = User.fetch( route.args.id )
      route.args.user.run(next)
    },
    function authAdmin (route, next) {
      if ( route.args.user.isAdmin ) {
        next()
      }
      else {
        route.redirect('/sign-in')
        // Alternatively, resolve here and mithril will
        // render the SignIn component under same route:
        //
        // route.resolve(SignIn)
      }
    },
    UserPage
    // ^ Plain component is equivalent to:
    //
    // function render (route) {
    //   route.resolve(UserPage)
    // }
  ],
  '/sign-in': SignIn
})

//
// Or, if functions were already defined elsewhere:
//
m.route(document.getElementById('app'), '/', {
  '/': Home,
  '/users/:id': [fetchUser, authAdmin, UserPage],
  '/sign-in': SignIn
})

This code makes perfect sense if you make these correlations:

• route.redirect() correlates to Express's res.redirect()
• route.resolve() correlates to Express's res.send()
• next allows functions to behave like Express's middleware

Once a route has resolved, that component will be mounted for redraws.

What this design effectively does is make the router solely concerned with the onmatch portion of the current API and removes the half-responsibility of render, all while supporting async things in a pleasantly modular fashion (via composable middleware).

[barneycarroll]

I like the more holistic route focus and getting rid of ambiguous constructs, but it's important to acknowledge the stated purpose for render as distinct from a view was that it would try to diff, whereas components wouldn't. I'm not sure how important that really is in the grand scheme of things (and I don't think it's all that intuitive or elegant), but is that functionality catered for here?

[gilbert]

Hmm, that's a good question. I would think that mapping routes to views would be the only thing you need. I guess I need an example of when one would _not_ want to use a component to give a proper answer.

[pygy]

The main use case is to provide a layout that isn't torn down and rebuilt from scratch on route change, i.e.

{
  '/a': {render: function() { return m(Layout, m(A)) }},
  '/b': {render: function() { return m(Layout, m(B)) }}
}

where the layout is diffed vs

{
  '/a': {view: function() { return m(Layout, m(A)) }},
  '/b': {view: function() { return m(Layout, m(B)) }}
}

where it is recreated since /a and /b point to different components.

[gilbert]

I see, thank you for the illustration. Could this perhaps be a weakness of components? Maybe components need a way to explicitly identify themselves, instead of solely relying on object identity:

{
  '/a': { id: 'layout', view: function() { return m(Layout, m(A)) }},
  '/b': { id: 'layout', view: function() { return m(Layout, m(B)) }}
}

This would support component constructors:

{
  '/a': withLayout(A),
  '/b': withLayout(B),
}

function withLayout (Component) {
  return { id: 'layout', view: () => m(Layout, m(Component)) }
}

Although I have no idea how easy or difficult it would be to support this in the render engine, I do believe this is a component + render problem and not a router problem. The router does need a way to decide when to redraw from scratch, though.

[pygy]

It would be easy to add AFAICT: turn this line into

        if (oldTag === tag || oldTag.id != null && oldTag.id === tag.id) {

The perf cost would have to be assessed, though, but it may be minimal.

Edit: For redrawing from scratch, you could use keys... Right now, the router doesn't set the vnode.key based on args.keys for reliability (I suppose), because the args can come not only from the route :variables but also from the URL ?query=parameters which are not controllable by the frontend dev (external links come to mind, or a server with fixed params). That could be solved by putting the query params in a distinct object (as already suggested elsewhere). The onmatch could then set args.key when necessary.

Edit2: @mindeavor I'll sleep on it but at first sight, that looks like a good suggestion, I think you're getting somewhere... Now we may have to convince @lhorie

[barneycarroll]

Very good idea! Can I suggest is (similar to custom elements)? id is confusing because of the ubiquity of the DOM property of the same name, which is usually used to identify _instances_ rather than 'classes'.

[gilbert]

I agree, it should be something different. Maybe componentId, to even further avoid confusion?

[isiahmeadows]

@barneycarroll I think is is a little too implicit and undescriptive ("is type" vs "is instance"), even though I tend to favor implicit over explicit.

[iamjohnlong]

Here is my proposal for route resolvers https://jsfiddle.net/iamjohnlong/0xqgkk2o/

https://github.com/iamjohnlong/mithril.js/blob/feature/router-middleware/api/router.js

I'm sure I'm missing some pieces though. Thoughts? Middleware can be call whatever feels more appropriate.

[tivac]

@lhorie Are any of the proposals in this thread something you're interested in tackling? Lack of response indicates "no" to me and I'd prefer to close this & say "RouteResolvers are what they are" if possible.

[lhorie]

@tivac I might consider additions post 1.0 if there's enough demand, but breaking changes at this point get a no.