Allow overriding RoutingContext instead of createElement

[taion]

Looking at the integration points into Router, it seems like it would make more sense to have the RoutingContext be the point for customizing the router's render behavior, rather than createElement.

For example, for integrating with Relay, one thing I want to do is to inject a Relay.RootContainer at top level. This is awkwardly possible right now using context (the element that doesn't get the context is the root element), but it'd be a lot easier if I could just control my own RoutingContext rendering.

[acdlite]

I agree this is a good idea. RoutingContext was designed exactly for the kind of integration you're talking about. We use it in Redux React Router to render an router app for a given state (which in our case, we manage with a Redux store): https://github.com/acdlite/redux-react-router/blob/next/src/ReduxRouter.js#L60-L64

That's sufficient in our case, because we don't need the state management provided by the top-level Router component. However, it will often be the case that you want to customize how RoutingContext is created while still using Router to manage state.

Perhaps an API like this:

// Default
function createRoutingContext(RoutingContext, props) {
  return <RoutingContext {...props} />;
}

// Or e.g. with Relay
function createRoutingContext(RoutingContext, props) {
  return (
    <RootContainer
      Component={RoutingContext}
      ...
    />
  );
}

<Router createRoutingContext={createRoutingContext} />

where props is essentially {...routerState, history}.

However, this doesn't replace createElement(), which supports creating multiple components.

[taion]

I think this would supersede createElement, in that createElement can be a prop of the default RoutingContext, rather than something exposed on Router itself. If support exists for customizing the RoutingContext, I think it'd be redundant and un-parsimonious to expose this as a prop on Router, just to pass it down to RoutingContext.

[ryanflorence]

<Router>
  <Route component={RelayContainerAtTheTop}>
    {/* more routes */}
  </Route>
</Router>

[taion]

That's a bit of an awkward API, no? I feel like it should be more like

<Router createRoutingContext={ReactRouterRelay.createRoutingContext}>
  {/* actual routes */}
</Router>

Currently I have:

<Router createElement={ReactRouterRelay.createElement}>
  {/* actual routes */}
</Router>

And that feels more right than adding a new artificial route.

[acdlite]

@ryanflorence The problem with that is owner-based context, although with 0.14 imminent I suppose that doesn't matter.

[mjackson]

You want to create your own <RoutingContext>, @taion? You'd have to duplicate everything that's going on in RoutingContext's render method if you did that...

[taion]

I just had a chat with @ryanflorence about this. The idea would have been that I wrap React Router's RoutingContext in another component, e.g. have something like

import createElement from './createElement';
import RootComponent from './RootComponent';
​
export default function createRoutingContext(props) {
  return (
    <RootComponent {...props}>
      <RoutingContext
        {...props}
        createElement={createElement}
      />
    </RootComponent>
  )
}

However, on second thought, this seems to not be worth it. createElement is a simpler API, and allows e.g. composition of multiple createElement functions.

Additionally, the amount of work required to inject a root component when just using createElement is negligible - it's literally just https://github.com/relay-tools/react-router-relay/blob/v0.4.4/src/Container.js#L21-L23, which is not worth complaining about.

So I'm going to close this for now and keep doing what I was doing.