[wip][added] Routing middlewares

[taion]

TODO: Add docs if we like this feature

Closes #2088

I decided to get this out there for visibility before spending time adding explicit docs. The new test case shows this in action, including a bit of an unavoidable quirk.

There's a few things behind this idea:

• createElement is not ideal as a point for general customizability because it's not composable - a user doesn't get anything to allow e.g. using both ReactRouterRelay.createElement and some other custom createElement function
• It's possible to inject additional root components with createElement, but it's fairly annoying to do so: https://github.com/relay-tools/react-router-relay/blob/v0.7.0/src/Container.js#L20-L23 (i.e. expose something on child context, and check whether that thing is there - if not, you must be on the root)
• This allows the routing middleware to access more data about the current router state (i.e. both components and routes), while not forcing us to expose those as props to all route components, which shouldn't need access to all of those (i.e. we could revert the commit fixing "branch" (or "routes") in beta4 #1788 and remove routes from the props injected to route components again)

Let me know what y'all think.

[taion]

@ryanflorence If this PR is accepted, I would like to drop routes from the props injected into route components. Would that break you for doing async props, or would you be able to model those with one of these middlewares?

[timdorr]

I think this should be held to post-1.0. We keep breaking APIs on a release candidate, so we're never going to actually release something. I know there are folks waiting on a 1.0 stable.

[taion]

This isn't a breaking API change, though - this is purely additive, so it would be a minor version bump per semver.

The reason I care is that this or something like it that allows access to the array of components is a pre-requisite for relay-tools/react-router-relay#12.

[mjackson]

In general I think asking people to provide a middlewares array is a worse pattern than asking them to do function/component composition. I've implemented the middleware pattern in the history library using the use* functions. Could what you're doing here be accomplished using this pattern?

[taion]

Thinking about this a bit more, the middlewares array is not ideal. Maybe a better API could be:

<Router history={history}>
  <Middleware1>
    <Middleware2>
      {routes}
    </Middleware2>
  </Middleware1>
</Router>

With RoutingContext this might look like:

<RoutingContext {...renderProps}>
  <Middleware1>
    <Middleware2 />
  </Middleware1>
</RoutingContext>

I don't think I can get all of what I want with a history enhancer. The issue for react-router-relay and generally anything that wants to manage data fetching at the top-level is that it needs to do a couple of different things:

1. Add a top-level "root" component that receives props like the full list of routes and components to figure out all the data dependencies for all the loaded routes (and e.g. dispatch them all at once)
2. Modify the createElement currently in RoutingContext to add a container around each of the route components to inject in the relevant data props

I believe both react-router-relay and @ryanflorence's async props prototypes work in a similar way.

It might be possible to get something like (1) via a history enhancer, but I think getting (2) requires something like this (for modifying createElement).

[taion]

The other motivation here is to move away from the createElement API for libraries extending React Router - while a user who only wants to use e.g. ReactRouterRelay.createElement can do so, a user who wants to use two such libraries seems like he or she would largely be out of luck.

Something like a composable middleware pattern for modifying element rendering would address that concern and offer a better entry point for libraries.

[taion]

Correction - the server-side example should perhaps be:

<Middleware1 {...renderProps}>
  <Middleware2>
    <RoutingContext />
  </Middleware2>
</Middleware1>

[taion]

This approach is dumb. I'm dropping this PR and taking a different approach.

[timdorr]

I think the core problem is trying represent a functional concept like middleware with JSX. There are some similarities in terms of nesting, but significant differences in how you might code something like that.

I think what you're looking for is something closer to redux's version of middleware, which is just an enhancer. So you would end up with something like this:

import { Router, applyMiddleware } from 'react-router'
import someMiddleware from 'react-really-cool-shit'
import someOtherMiddleware from 'react-quite-neat-thing'

const RouterPlus = applyMiddleware([
  someMiddleware,
  someOtherMiddleware({ option: 123 })
], Router)

React.render(
  <RouterPlus>
    <Route path="/" component={App} />
  </RouterPlus>,
  rootDOM
)

[taion]

The issue in some sense, and the difficulty around all of this is that I'm trying to apply middleware to the <RoutingContext>, not the router.

Short of doing something really nasty with ES6 inheritance, I can't meaningfully get what I need by modifying the <Router> component itself.

I'm going to submit another, simpler PR shortly.

[ryanflorence]

I built this a bit ago, I really dig it, haven't had a chance for show and tell to get people's feedback on it though:

https://github.com/ryanflorence/react-router-apply-middleware

[timdorr]

@ryanflorence That's pretty cool. Why don't we have that in core? 😄

[taion]

I really like the API – it's much cleaner than the fiddly stuff I have in e.g. react-router-relay, and avoids an issue with route handler containers getting applied in the opposite order.

Maybe we should discuss this on a new issue instead of a closed, half-year-old PR though 😆

[timdorr]

Maybe we should discuss this on a new issue instead of a closed, half-year-old PR though 😆

Or even better, a PR to add it 👍