[1.0] Add ability to provide custom render methods on routes.

[cpojer]

This allows to inject a custom "route handler" (hah!). This is a useful hook to render something different from react-router's default call to React.createElement. This method will be called for every component on every step of the route tree and it can conditionally return a ReactElement. This is useful when building an application with Relay.

Actual Code Example:

function render(component, {params, route}) {
  var {name, queries, component} = route;
  if (name && queries && Relay.isContainer(component)) {
    return (
      <RelayRootContainer
        Component={component}
        route={{name, params, queries}}
      />
    );
  }
}

var routes = (
  <Route component={Root} render={render}>
    <Route …>
      <Route name="app" component={App} queries={{…}} />
      <Route name="profile" component={Profile} queries={{…}} />
      <Route name="events" component={Events} queries={{…}} />
    </Route>
  </Route>
);

The application code is not affected by this; components will just receive this.props.children.

It is possible that such a render function might be useful on more steps than just the top level. In this case I would consider changing my code to walk up the matched route tree and select the first render method that is defined. If we do this, we might also want to consider walking up the matched route tree and calling all render methods (closest ones first) until one of them renders to an element.

[josephsavona]

+1

[cpojer]

Actually I like the idea about walking up the matched routes and calling render until one of them returns a valid element. It seems like it provides the most option value. What do other people think?

[devknoll]

Just curious, would this be essentially equivalent to this?

BrowserHistory.listen(function (location) {
  Router.match(location, function (error, props) {
    props.components.each(component => render(component, ...));
    React.render(<Router {...props}/>, document.body);
  });
});

The discussion in #1211 might be relevant too 👍

[cpojer]

No it is not because props.components are the matched component constructors. Also it would be a mutative API which I don't want to encourage people to use.

[ryanflorence]

Any reason App doesn't render a RelayContainer instead? you have this.props.route available.

[cpojer]

I've updated the example to include more than one route on the same level. Yes, what you are saying works but it is really inconvenient. It means that the boilerplate code from the render function above needs to be written out manually for every component.

Relay uses a RelayRootContainer as an anchor point for a Relay app or part of a Relay app. A RelayRootContainer takes a component and a query config (a route with {name, params, queries}).

So without this pull request, the App component would look something like this:

App.render = () => {
  return (
    <RelayRootContainer
      Component={AppContainer}
      route={{name, params, queries}}
    />
  );
};

but then Profile.render and Events.render would look identical except for the component name. This is clearly not optimal as you might have 100 routes. Even App.render = createRelayRendererFor(App) is inconvenient as it requires the developer to type out this level of indirection manually every time. It also makes it more work to convert existing views to Relay one-by-one.

Now another solution could be something like this:

<Route name="app" component={RootContainerWrapper} relayComponent={App} queries={{…}} />
<Route name="profile" component={RootContainerWrapper} relayComponent={Profile} queries={{…}} />

but I think this is worse. It means that all the async loading logic you guys built won't work the same way. If you look at this example closely it is actually quite similar to the solution in this pull request. Except with my pull request (and the suggestion I posted above about picking the closest render) we could retain component for the actual component (a RelayContainer) and we can use render on the closest level where we automatically want to wrap components with a RelayRootContainer. In the app I'm currently using this with it works really well.

[ryanflorence]

is routes[0] always the first route? or is it "this route"?

[ryanflorence]

Sorry, not against this idea, just making sure we're thinking about acceptable alternatives.

var routes = (
  <Route component={createRelayContainer(Root)}>
    <Route …>
      <Route name="app" component={App} queries={{…}} />
      <Route name="profile" component={Profile} queries={{…}} />
      <Route name="events" component={Events} queries={{…}} />
    </Route>
  </Route>
);

Seems to have the same characteristics of what you're talking about, or I'm missing something completely.

[cpojer]

It could work if we did something like this:

function wrap(component) {
  return class extends React.Component {
    render() {
      var {params, route} = this.props;
      var {name, queries, component} = route;
      if (name && queries && Relay.isContainer(component)) {
        return (
          <RelayRootContainer
            Component={component}
            route={{name, params, queries}}
          />
        );
      }
    }
  }
}

var routes = (
  <Route component={Root} render={render}>
    <Route …>
      <Route name="app" component={wrap(App)} queries={{…}} />
      <Route name="profile" component={wrap(Profile)} queries={{…}} />
      <Route name="events" component={wrap(Events)} queries={{…}} />
    </Route>
  </Route>
);

which is fine if you look at it like this. But the problem is now that on every route change you are wiping away the whole React tree because every route has its own wrapper component and React doesn't understand that they are doing the same. This means route transitions are more expensive than they have to be.

I think giving developers the ability to modify the default children makes sense from an API perspective. Just like it was possible previously to build your own <RouteHandler />.

[cpojer]

I updated the pull request with a new version that calls render on every route up the stack from the current one. I also added a test for this behavior, which gets a little complex but it covers all the cases.

The way it works now is that routes can define render on any level and the first render method that returns something will be used for children.

Example Nested Routes: RouteA -> RouteB -> RouteC. Assume every route has a render method: renderA, renderB, renderC. When creating children for RouteC it calls renderC. If it doesn't return anything it calls renderB. It goes up the tree until one of them returns a React element. If none of them returns anything it falls back to the default behavior of react-router.

When creating children for RouteB it will call renderB and renderA (assuming routeB doesn't return anything). For RouteA's component it will only call renderA.

[wincent]

s/renderes/render/ (or your preferred spelling of choice)

[devknoll]

Just to throw out another random idea...

var routes = Relay.wrapRoutes(
  <Route component={Root}>
    <Route …>
      <Route name="app" component={App} queries={{…}} />
      <Route name="profile" component={Profile} queries={{…}} />
      <Route name="events" component={Events} queries={{…}} />
    </Route>
  </Route>
);

then Relay.wrapRoutes could use createRoutesFromReactChildren to get a tree and post-process it, wrapping any components it needs to.

This definitely doesn't address your route transition concern and would put extra strain on your lib to handle async routes/components... but just throwing out ideas.

[devknoll]

Another crazy idea: let createElement be passed as a prop to Router instead. Then all of the wrapping functionality can exist as a Relay HoC... i.e.

var routes = createRouter(
  <Route component={Root}>
    <Route …>
      <Route name="app" component={App} queries={{…}} />
      <Route name="profile" component={Profile} queries={{…}} />
      <Route name="events" component={Events} queries={{…}} />
    </Route>
  </Route>
);

React.render((
  <Relay.Router>
    <Router history={BrowserHistory}/>
  </Relay.Router>
), document.body);

Seems like that could address the route transition concern and be more flexible to boot.

[cpojer]

It is unlikely that we are going to ship something like this as part of
Relay. Relay is not opinionated about any routing system that you may want
to use. While your suggestion might work, it would be specific to the two
libraries and react-router would need to expose the internal functions that
deal with different kinds of routing objects.

The solution in this PR is generic and can work with anything. Even if we
decide against using this, a way to overwrite the children that
react-router creates will definitely be requested by others. It's also a
feature that exists in 0.13 via <RouteHandler />, which already can be
overwritten. My initial plan was to build <RelayRouteHandler /> but this is
way more generic.

Unless I'm missing something, the second example seems to match the first iteration of this PR – a way to overwrite createElement, except only on the top level. I decided to change it to visit every route in the stack because the top level route shouldn't be special cased.

[agundermann]

But the problem is now that on every route change you are wiping away the whole React tree because every route has its own wrapper component and React doesn't understand that they are doing the same. This means route transitions are more expensive than they have to be.

Are you sure about that? The component classes are only defined once upfront, so as long as only some inner routes change, the components of the outer routes should be preserved, shouldn't they? I can see that it makes a difference when transitioning to a sibling route with the same component class though.

I've felt like the Router run callback was the right place to do this kind of processing on route changes, passing everything that's needed as props to Router (similar to @devknoll 's suggestion). But with the upcoming API changes, and the addition of named routes, this render() approach might be more reasonable.

I wonder if it would be a good idea to also use it for passing async data from the change callback

function render(component, {params, route, passProps}){
  var data = passProps[route.name]; // example mapping..
  return React.createElement(component, { params, route, data }); 
}

BrowserHistory.listen(function (location) {
  Router.match(location, function (error, props) {
    fetchData(props, function(err, data){
       React.render(<Router {...props} passProps={data} />, document.body);
    });
  });
});

[gaearon]

There are some changes to 1.0 API which might actually make all of this way more straightforward. When @ryanflorence comes out of the API design cage he'll tell the world. With this new not-yet-shown API, it might be possible to implement Relay integration as a middleware component. (Stay tuned!)

[ryanflorence]

With the new architecture this is kind of ridiculously easy now, no pull requests needed!

https://github.com/rackt/react-router/blob/next-ryan/doc/04%20Middleware/RouteRenderer.md