+1

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?

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 👍

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

Any reasonApp doesn't render aRelayContainer instead? you havethis.props.route available.

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 therender function above needs to be written out manually for every component.

Relay uses aRelayRootContainer as an anchor point for a Relay app or part of a Relay app. ARelayRootContainer 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(<RelayRootContainerComponent={AppContainer}route={{name, params, queries}}/>);};

but thenProfile.render andEvents.render would look identical except for the component name. This is clearly not optimal as you might have 100 routes. EvenApp.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:

<Routename="app"component={RootContainerWrapper}relayComponent={App}queries={{…}}/><Routename="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 retaincomponent for the actual component (aRelayContainer) and we can userender on the closest level where we automatically want to wrap components with aRelayRootContainer. In the app I'm currently using this with it works really well.

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

varroutes=(<Routecomponent={createRelayContainer(Root)}><Route…><Routename="app"component={App}queries={{…}}/><Routename="profile"component={Profile}queries={{…}}/><Routename="events"component={Events}queries={{…}}/></Route></Route>);

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

It could work if we did something like this:

functionwrap(component){returnclassextendsReact.Component{render(){var{params, route}=this.props;var{name, queries, component}=route;if(name&&queries&&Relay.isContainer(component)){return(<RelayRootContainerComponent={component}route={{name, params, queries}}/>);}}}}varroutes=(<Routecomponent={Root}render={render}><Route…><Routename="app"component={wrap(App)}queries={{…}}/><Routename="profile"component={wrap(Profile)}queries={{…}}/><Routename="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 />.

I updated the pull request with a new version that callsrender 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 definerender on any level and the firstrender method that returns something will be used for children.

Example Nested Routes:RouteA -> RouteB -> RouteC. Assume every route has arender 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.

Just to throw out another random idea...

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

thenRelay.wrapRoutes could usecreateRoutesFromReactChildren 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.

Another crazy idea: letcreateElement be passed as a prop toRouter instead. Then all of the wrapping functionality can exist as a Relay HoC... i.e.

varroutes=createRouter(<Routecomponent={Root}><Route…><Routename="app"component={App}queries={{…}}/><Routename="profile"component={Profile}queries={{…}}/><Routename="events"component={Events}queries={{…}}/></Route></Route>);React.render((<Relay.Router><Routerhistory={BrowserHistory}/></Relay.Router>),document.body);

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

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 overwritecreateElement, 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.

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

functionrender(component,{params, route, passProps}){vardata=passProps[route.name];// example mapping..returnReact.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);});});});

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!)

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