make Router a React component

[josephsavona]

Is there a specific motivation for making Router() not be - ie a standard, renderable React component? This would seem to make server-side rendering easier as you can tie in directly to React's rendering capabilities.

[ryanflorence]

Heh, that's exactly why it's not :)

We are still working on server side rendering, Michael can speak more intelligently about it than I can.

On Jun 20, 2014, at 10:01 AM, Joseph Savona [email protected] wrote:

Is there a specific motivation for making Router() not be - ie a standard, renderable React component? This would seem to make server-side rendering easier as you can tie in directly to React's rendering capabilities.

—
Reply to this email directly or view it on GitHub.

[mjackson]

@josephsavona There was a point when it was a React component, but then we gave it too many responsibilities so it wasn't a good fit anymore. However, a lot of its responsibilities now live in Route.js (not components/Route.js) so I've been thinking the same thing. It could possibly work as a regular React component now. It's actually something I'm planning on looking into very soon.

Also, please keep in mind as you're looking through this code that both @rpflorence and I are Ember guys making the transition to React, so any pointers you have to offer are most welcome. ;)

[mjackson]

Ah, I just remembered why the router isn't a component. It has to do with links.

The <Link> components need to know their hrefs, and in order to do that, they need to be able to lookup the Route they're linking to by name. So we can't wait until a Router component mounts to find out what the routes are, unless we treat the available set of routes as data, instead of configuration.

I'm working on a solution to this, but that's where it is right now.

[mjackson]

OK, I've got a branch that uses a regular React component for the router here: https://github.com/mjackson/react-nested-router/tree/router-component

The benefits of this are significant enough to break with the existing API. For one, we don't have to have our own equivalent of React's top-level render* methods, which may be the issue behind #34. Also, making the router a regular component lets us easily pass multiple children, so this could address #33 as well.

[ryanflorence]

if it is a component, and we call renderToString on it, do we ever get a chance to make it async so stores can be primed with data from component transition hooks?

[josephsavona]

I wondered about this as well. Originally the renderToString API was async which might have made this case easier, I'm not sure why that changed.

[josephsavona]

@rpflorence do the transition hooks have to fire for the initial rendering pass? Or is the intention that parent components can asynchronously provide data to a nested ? I wonder if there's a way to abstract the nesting hierarchy from React, so that you can walk the tree for transition calls first, and then make the render call separately.

[ryanflorence]

@josephsavona A couple of things to consider first that I think are important:

1. View hierarchy should not be necessarily coupled to data hierarchy
2. Route handler components should own their data so they are portable

My strategy for server-side rendering has been to call the transition hooks before rendering, and in them prime the stores the components use with the right data, then when the render happens, the stores can immediately return data to the components. Then the only code you need to change to get server-side rendering to work is to prime the store in the transition hooks.

I'm not sure if this is on the right track for the react way™, but I really think keeping view hierarchy and data hierarchy decoupled is important.

[mjackson]

@rpflorence I agree we should keep view hierarchy decoupled from data.

The only hitch I can see with the approach you suggest is that people don't typically use flux-style stores in server environments.

Perhaps we could expect some kind of return value from willTransitionTo, and keep it in a big bag somewhere that we use during the render. A sample server-side request handler might look like this:

function (request) {
  return route.dispatch(request.path).then(function (bunchOfData) {
    return React.renderComponentToString(route.props.handler(bunchOfData));
  });
}

[ryanflorence]

This approach requires data to come in as a prop from the parent which feels like coupling view hierarchy with data.

What does component code look like with this? Get state from some data prop or go fetch it if it's not there?

I anticipate this conversation coming back to the context stuff we used to have, and maybe that's the right solution...

Sent from my iPhone

On Jun 21, 2014, at 4:16 PM, Michael Jackson [email protected] wrote:

@rpflorence I agree we should keep view hierarchy decoupled from data.

The only hitch I can see with the approach you suggest is that people don't typically use flux-style stores in server environments.

Perhaps we could expect some kind of return value from willTransitionTo, and keep it in a big bag somewhere that we use during the render. A sample server-side request handler might look like this:

function (request) {
return route.dispatch(request.path).then(function (bunchOfData) {
return React.renderComponentToString(route.props.handler(bunchOfData));
});
}
—
Reply to this email directly or view it on GitHub.

[mjackson]

Yeah, I was just going to mention the context stuff.

@josephsavona We used to have a feature that let you access the return value of willTransitionTo in your component as this.props.context. So, for example, you could have a User component that did something like this:

var User = React.createClass({
  statics: {
    willTransitionTo: function (transition, params) {
      return fetchUserFromTheDatabase(params.id);
    }
  },
  render: function () {
    var user = this.props.context;
    return <div>Hello {user.name}!</div>;
  }
});

Ultimately we decided to remove this feature because it didn't quite feel right. The user doesn't technically "own" this.props.context, so they shouldn't be updating it. But they will want their component to update as new data becomes available, so how do they do that without updating the context?

One way that might work is to use the context to initialize the state of a component, but then throw it away. The React docs specifically point out that using props to initialize state is an anti-pattern, but they do point out that

it's not an anti-pattern if you make it clear that synchronization's not the goal

so that's basically what we would be doing. The context could be used as the starting point for a component's data, but it shouldn't be considered an enduring source of truth. On the server this gives us the data we need for a one-pass render. On the client, this gives us a starting point that we can use on the first render until we get more data.

One major caveat with this approach is that the router currently waits until all transition hooks resolve before updating the UI. This would definitely lead to some lag time on clients that need to fetch data from the server, which is another big reason we removed this feature. In these cases, it seems better to update the UI immediately and show some "loading..." message.

The other major caveat with this approach is it overloads the purpose of the willTransitionTo hook. Right now its only purpose is to either transition.abort()/transition.redirect(), which is why we wait until it resolves before updating the UI. If we determine it's also the place to load data it becomes unclear how to use it.

The other approach would be to punt on the data loading entirely and recommend a flux-style data store solution that works in a server-side scenario, but I'm not sure yet what that looks like.

[matthewwithanm]

@mjackson Have you talked to @andreypopp about this at all? What you're describing sounds a lot like getInitialStateAsync in his react-async project.

[petehunt]

I think if this.props.context were renamed to this.props.initialData this would make a lot of sense. On the server it will block until willTransitionTo() completes (see #36) and will render an HTML document with the data intact. If you then render on the client with the initialData prop as fetched from willTransitionTo() (basically, we'd change the signature of renderComponentToString() to return a tuple of (html str, initialData) and have the router take an optional initialData on the client) the client would be able to attach to the markup.

If you don't server render, then the client will render with an empty initialData while it waits for willTransitionTo(). Though this changes the semantics of willTransitionTo(): it can no longer block render.

[josephsavona]

@mjackson i suppose my concerns were twofold: one, supporting server-side rendering - @petehunt's idea of returning (html, data) makes a lot of sense here - but also more conceptual.

My main concern with the library's direction is about lock-in. For me, the lesson from react is that libraries should aim to do just one thing and do it well, as react does for views. Nested routing is a highly useful concept that applies to any rendering library: it would be great to be able to mix some legacy EJS or handlebars templates into a larger React app, for example. What you and @rpflorence have with react-nested-router seems to be a great start on two pieces of this: an awesome nested router (which takes advantage of JSX to for intuitive DSLs) as well as a component that can take a nested routing description and render it. I wonder if you've considered the idea of separating the library along those lines.

For example, we've been working on something like the following (but supporting only a single level of routes):

<Route handler={AppHandler} defaultRenderer={ReactRenderer}>
  <Route name="home" handler={HomeHandler} />
</Route>

AppHandler = Handler({
  template: AppComponent, // <- React.createClass(...)
  initialize: function() {
    this.setProp('propName', propValue); // <- becomes available in component as this.props.propName
    this.setProp('asyncProp', promiseReturningFn()); // <- renderer will wait until promise resolves and then set value of prop
  }
});

HomeHandler = Handler({
    template: function() {}, // <- compiled handlebars
    renderer: require('handlebars'), // <- specify custom renderer
    initialize: function() {
      this.setProp('homeProp', promiseReturningFn());
   }
});

In this model a renderer is given a structure (parent -> child -> ... -> leaf) representing the matching urls handlers, and it must be able to render as either a string or into the DOM.

Again, these are just food for thought and in no way a critique - love what you're doing with the library and it's just got us thinking.

[ryanflorence]

Thanks @josephsavona :)

The circle you draw around a set of features that qualifies as "one thing" is pretty subjective :) I consider this library to do one thing: couple react view hierarchy to the url.

But, it looks like server-side rendering is nudging us toward prescribing how to provide data to route handlers as well. Whatever the solution, I don't want it to be too prescriptive, just enough to allow for both rendering scenarios and nothing more.

[mjackson]

@matthewwithanm Thank you for the link to react-async. I think @andreypopp is taking the right approach there to async data loading; namely, that you need to have a render that can render while waiting for async data and you also need an async renderComponentToString so you can render async components on the server. Plus, this solution lets us focus on one thing, routing, and decouple ourselves from loading data. I'm leaning toward recommending this solution at the moment.

@petehunt willTransitionTo needs to block render so that you can do things like prevent navigating away from a page with a form half filled-out. We can't render new UI while the user is looking at the confirm dialog, for example. This makes it feel like a bad place to load data from the server.

@josephsavona I would agree with @rpflorence that we're trying to do just one thing well, which is specifying a set of components that render with a given URL. There are a few things that help you do that more effectively, like transition hooks and transition.retry, but it's all focused on getting that one job done and making it convenient.

If you've got legacy Handlebars templates, why not simply create a mixin for components that need to use them and stuff them inside render?

BTW, everyone, our top-level API is now just a regular React component. Check the docs for update usage info.

[mjackson]

Given that the Route component is now a regular React component (which was the primary purpose of this issue) I'm going to close. If you'd like to continue discussing loading data, let's do it in another issue.

[josephsavona]

@mjackson makes sense - thanks for the follow-up