Discussion about router

[johnsusi]

While developing a set of components I was surprised to see that the order of which render was called was not what I was expecting.

Here is a small example

class A
  include Clearwater::Component

  def initialize(children = [])
    @children = children
  end

  def render
    puts 'render A'
    div({id: 'A'}, @children)
  end
end

class B
  include Clearwater::Component

  def render
    puts 'render B'
    div({id: 'B'}, 'hello')
  end
end

class Layout
  include Clearwater::Component

  def render
    div({id: 0}, [
      A.new([
        div({id: 1}, [
          B.new
        ])
      ])
    ])
  end
end

Console output

render B
render A

However when I remove one nested div in Layout the order is reversed.

class Layout
  include Clearwater::Component

  def render
    div({id: 0}, [
      A.new([
        B.new
      ])
    ])
  end
end

Console output

render A
render B

Looking at https://github.com/clearwater-rb/clearwater/blob/master/opal/clearwater/component.rb#L55 it is clear why it is working this way. I see no easy fix so perhaps this is just something that needs documenting.

Or do you consider Router to be the only way to nest components properly?

I guess this is not a problem in most usecases.

[johnsusi]

Maybe it is better to use blocks for nesting. Then render is called consistently for any type of hierarchy.

class A
  include Clearwater::Component

  def initialize(&children)
    @children = children
  end

  def render
    puts 'render A'
    div({id: 'A'}, @children.call)
  end
end

class Layout
  include Clearwater::Component

  def render
    div({id: 0}, [
      A.new do
        div({id: 1}, [
          B.new
        ])
      end
    ])
  end
end

[jgaskins]

This is a little surprising, but can you think of a case where it would make a difference in how I would write a particular component?

[johnsusi]

I was trying out an idea where nested components needed to get information from a 'parent' higher up but passing down attributes was not an option.

React has context for this kind of thing. Nothing you use often but sometimes it makes sense. https://facebook.github.io/react/docs/context.html

The block style solved my issue so I'll close this ticket.

[jgaskins]

I was trying out an idea where nested components needed to get information from a 'parent' higher up but passing down attributes was not an option.

I'm not sure I understand. Do you mean like in your example above where you construct the A and B directly in the Layout?

I do something similar sometimes with a MasterDetail component:

MasterDetail = Struct.new(:master, :detail) do
  include Clearwater::Component

  def render
    div([
      div({ style: Style.master }, master),
      div({ style: Style.detail }, detail),
    ])
  end

  module Style
    module_function

    # style methods for above
  end
end

class Layout
  include Clearwater::Component

  def render
    MasterDetail.new(UserList.new(users), UserDetail.new(selected_user))
  end
end

[jgaskins]

I agree that some of this should be documented, btw. We're mixing function calls (the HTML tag methods) with objects whose values are delayed until the sanitization pass. And then there are CachedRender components whose results are delayed even further.

[johnsusi]

Interesting, I like the style of MasterDetail. Looks similar to 'pure components' in React.

I was toying with an idea of a declarative router similar to what they are doing in react-router v4.

  def render
    Router.new do
      div({ id: 'app' }, [
        header({ class_name: 'main-header' }, [
          h1('Hello, world!'),
        ]),
        Match.new(pattern: '/about',    component: About),
        Match.new(pattern: '/articles', component: Articles),
        Miss.new(component: Articles)
      ])
    end
  end

but execution order of render becomes a problem in nested matches. For instance Articles should match on '/:article_id'

  def render
    div({ id: 'articles-container '}, [
      input({ class_name: 'search-articles', onkeyup: method(:search) }),
      ul({ id: 'articles-index' }, articles.map { |article|
        ArticlesListItem.new(article)
      }),
      Match.new(pattern: '/:article_id', component: Article)
    ])
  end

And as you say, I haven't even begun to look at the effects of CachedRender :)

[jgaskins]

I love some of the ideas behind RR4 and I wondered, too, if it was possible to do something like they did. :-) There were a few things that kept me from being able to do it successfully:

• Lack of context, partially because of encouraging using MyComponent.new during render rather than a DSL method that would allow us to pass the context object through automagically
• RR4 completely inverts the component/router relationship in a way that Clearwater::Router can't support at the moment.
• Other stuff I can't remember at 3am.

I wonder if we could make it work with another hypothetical mixin:

class Layout
  include Clearwater::Component
  include Routing

  def render
    div([
      match('/articles') { Articles.new },
      match('/about') { About.new },
      miss { Articles.new },
    ])
  end
end

class Articles
  include Clearwater::Component
  include Routing

  def render
    MasterDetail.new(
      ul(articles.map { |article| ArticlesListItem.new(article) }),
      div([
        # Matching a dynamic segment could pass the value to the block
        match(':article_id') { |article_id| ArticlePage.new(articles[article_id]) },
        miss { h2('Please select an article from the left' },
      ])
    )
  end
end

The Routing mixin would provide the match and miss methods. I liked your idea of them being objects, but I can't think of a way to make Miss.new work differently based on whether a previous Match.new applied and scope it to the current component instance. :-)

If we can make this work, this opens up some really, really cool stuff, like match and miss returning CachedRender components of their own that also include the URL as part of its should_render? criteria, passing along their own context (which may be necessary to do some of the stuff outlined in this post), etc.

[johnsusi]

That looks good! I'll reopen and rename this issue so others can join in the discussion.

I have a rough implementation going at https://github.com/johnsusi/clearwater-crossroads/

It handles the simple cases but currently fails when on nested matches.

[jgaskins]

@johnsusi I played around a little with an idea today and got something working, too: https://gist.github.com/jgaskins/50b0bc5f0a8cea038e24d9d29dd66129#file-routing-rb

Also included example components using it. It doesn't match dynamic segments yet (that was the hardest part in Clearwater::Router, too), but there's a spot in the code where I think it can be added simply. It does support nested routes (it keeps a running check all the way down the tree of what's already been matched previously) and using multiple route blocks so you can match multiple times like in the RR Sidebar example (which it looks like yours might be able to do, too?).

We might be able to take some inspiration from Roda's router here.

[jgaskins]

Here's a full example Clearwater app with dynamic segments working. It's quite naïve atm, only letting you match one path segment (a segment being the part between the slashes in the path) at a time, though Clearwater's own router isn't much better (despite containing much more code). It'll match multiple segments at a time when the match contains no dynamic segments, though.

I'm getting a lot more excited about this the more I experiment with it.

[johnsusi]

I actually tried using the matcher from rails but I could not get it to work.

About nesting, https://github.com/johnsusi/clearwater-crossroads/blob/master/spec/crossroads/match_spec.rb#L70

That should actually work in opal but fails when running specs since the ruby version of the dsl runs the sanitize_content on to_html (ie outside of the routers render-method)

[johnsusi]

Another idea i like is to be able to inject the path at render time

def render
  router( '/foo/bar' ) {
    # the matcher would see '/foo/bar' now
    match('/foo') # ...
  }
end

Useful for testing and nesting components that know nothing of each other (except that they use routers)

It would also be nice to be able to match on query parameters, and here it might actually be wise to let multiple matchers succeed

def render
  div([
    match('?settings=open') { SettingsPanel.new },
    match('?toolbar=open') { Toolbar.new },
  ])
end

but I always have this nagging feeling that why not just do it in code :)

def render
  div([
    SettingsPanel.new if query[:settings] == :open,
    Toolbar.new            if query[:toolbar]  == :open,
  ])
end

[jgaskins]

These are awesome ideas, too. Making the path injectable is a great idea for testing.

If we can make this work with query params, that's something it'll have over Clearwater::Router. It doesn't do query params at all. I added a PR to Bowser for a URL parser that might make this easier. :-)