Remove support for nested absolute routes

[taion]

What do we think about dropping support for having nested routes with absolute paths? This will simplify route matching and remove the annoying inconsistency between sync and async routes, and will make support for things like condition per #3098 nicer.

If we look at the motivating example from https://github.com/reactjs/react-router/blob/v2.0.1/docs/guides/RouteConfiguration.md#decoupling-the-ui-from-the-url:

render((
  <Router>
    <Route path="/" component={App}>
      <IndexRoute component={Dashboard} />
      <Route path="about" component={About} />
      <Route path="inbox" component={Inbox}>
        {/* Use /messages/:id instead of messages/:id */}
        <Route path="/messages/:id" component={Message} />
      </Route>
    </Route>
  </Router>
), document.body)

We could better write this as:

render((
  <Router>
    <Route path="/" component={App}>
      <IndexRoute component={Dashboard} />
      <Route path="about" component={About} />
      <Route component={Inbox}>
        <Route path="inbox" />
        <Route path="messages/:id" component={Message} />
      </Route>
    </Route>
  </Router>
), document.body)

And in general I believe this sort of pattern is going to be possible – and it just makes it more clear that matching can only continue down routes that either actually match or are path-less.

[ryanflorence]

This was one of the primary use-cases that led me to creating react router. A previous router wouldn't let me do it, or at least I couldn't figure out how to, and I had to say "no" to our product team who wanted some urls that weren't nested.

Though, now that I type that, I wonder if this could work ...

<Route path="/" component={App}>
  <Route path="inbox" component={Inbox}/>
  <Route component={Inbox}>
    <Route path="messages/:id" component={Message} />
  </Route>
</Route>

Message is still nested beneath Inbox but has its own url.

Interesting ... I think I'm okay with that if it makes the matching code faster, easier to add new features to, or prevents bugs we can anticipate.

But, I'm not okay with it if it's just something that makes us as programmers sleep better at night knowing our matching algorithm is less complex.

[taion]

@ryanflorence

I think the second example I give in the OP is better, in that it doesn't duplicate component={Inbox}.

The main benefit from a code hygiene perspective is that it makes our route matching work in a much more intuitive way – specifically, we don't end up traversing down all possible routes, including ones that don't match, just because they might have nested absolute children.

The proximate motivation is to let us move forward with #3098 – at the moment, this has the potential to work in an extremely unintuitive and unpleasantly surprising way, since we keep following down routes that already haven't matched.

[ryanflorence]

Ah, I missed that in your comment, that does look good. It's way less intuitive for end-users, but in the case of async routing we would probably be saving a lot of network activity as well by simplifying the matching.

(Sorry for missing that, I'm actually making a github.com/notifications replacement, and it just hit the "almost useful" stage so I commented :P)

[taion]

For the good the async case, to minimize going down branches that won't eventually lead to a match, something like what you have is actually better, though you'd probably want to pivot it a bit like:

<Route path="/" component={App}>
  <Route path="inbox" component={Inbox} />
  <Route path="messages/:id" component={Inbox}>
    <IndexRoute component={Message} />
  </Route>
</Route>

But per #2244, nested absolute routes don't work with async routes anyway, so whatever we do here has no bearing on how users of async routes handle things.

Anyway, given that my PR to replace our route matching with path-to-regexp (#3105) – and more broadly any substantive changes to how we define path strings – is blocked on figuring out how we want to want to warn for unreachable routes (#1923), the only way to move forward on user-customizable route matching (#2286) is via adding a per-route binary check (#3098), but that check is going to work in a really unintuitive way without this change.

[timdorr]

I'm also for this. I think it's actually more confusing to the end user that they can create paths that don't match the layout of their route tree. You can always make that route structure work in another way anyways, even if it's a bit more verbose.

@taion Dude, your issue reference game is on point today 😆

[johanneslumpe]

@taion I think it's valid to remove support for this. When I originally posted #2244 I thought it might be a good idea since the router supported paths which broke out of their nested structure. Since then I actually never needed this and @timdorr has a good point on it being more confusing.

I'm wondering if a lot of people depend on being able to use nested absolute routes in general. If not, then I'm all for removing this.

[ryanflorence]

My hunch is that it's not used that often, and since we can create the same paths with the components with some finessing around route config, I'm happy to forward. @mjackson, thoughts?

[mjackson]

I'm actually really optimistic about this. Nested absolute routes are one feature that really prevented me from simplifying the implementation a lot. It's been a while since I've loaded it all into my head, but I think there are some really cool things we can do if we get rid of that constraint.

However, as @ryanflorence said, we absolutely need the ability to nest UI at "root" URLs. That was the whole motivation behind the shared-root example in the first place. The decoupling of route nesting from URL nesting is something I'm not willing to part with.

I'll think about this over the next few days, but I'm optimistic we can move forward here. Seems like we can just juggle things around in the route config w/o nested absolute paths to make any scenario that is possible with them.

[ryanflorence]

Seems like we can just juggle things around in the route config w/o nested absolute paths to make any scenario that is possible with them.

Yeah, I was screwing around the other day and I couldn't figure out a scenario that isn't possible.

DELETING CODE IS THE BEST

[taion]

The simple scenario above is easy to deal with. The trickier one is something like:

<Route path="foo" component={Foo}>
  <Route path="bar" component={Bar}>
    <Route path="/baz" component={Baz} />
  </Route>
</Route>

I think you end up having to declare this as something like:

<Route component={Foo}>
  <Route path="foo" />
  <Route component={Bar}>
    <Route path="foo/bar" />
    <Route path="baz" component={Baz} />
  </Route>
</Route>

Or else:

<Route component={Foo}>
  <Route path="foo">
    <Route path="bar" component={Bar} />
  </Route>
  <Route component={Bar}>
    <Route path="baz" component={Baz} />
  </Route>
</Route>

In other words, there's no way to do this without some duplication, either in path or in component definitions.

But IMO this example is contrived, and to do this in the first place is just confusing – plus I've demonstrated workarounds here anyway.

[taion]

Anyway, the only actionable thing here is to add a warning.

[ryanflorence]

Second example is how I'd do it, and document it, there really isn't any "duplication", it just seems like there is. It's actually more clear than a magical / I think.

[mjackson]

What would be really nice is if we had a build step for route configs.
Then, we could generate code in scenarios like this, instead of telling
users to do it themselves/putting it in the docs.

On Mon, Mar 14, 2016 at 9:22 AM Ryan Florence [email protected]
wrote:

Second example is how I'd do it, and document it, there really isn't any
"duplication", it just seems like there is. It's actually more clear than a
magical / I think.

—
Reply to this email directly or view it on GitHub
#3172 (comment)
.

[taion]

I don't think having something like a build step is possible in general – getChildRoutes currently supports doing fully dynamic things, so we can't know statically what the routes are.

[mjackson]

It absolutely is- just need to think outside of the paradigm we're
currently working in :)

I'm thinking more along the lines of a bundler plugin (probably start w
webpack) that can analyze your route config and generate all those config
objects. Asking users to programmatically configure the router is tedious.

On Mon, Mar 14, 2016 at 9:41 AM Jimmy Jia [email protected] wrote:

I don't think having something like a build step is possible in general –
getChildRoutes currently supports doing fully dynamic things, so we can't
know statically what the routes are.

—
Reply to this email directly or view it on GitHub
#3172 (comment)
.

[taion]

That's not what I mean. getChildRoutes is an opaque function that can return literally anything. It doesn't even need to be deterministic.

Maybe I'm misunderstanding what you mean. What is your preferred API for defining the route configuration above?

[taion]

Heads-up: I plan to close this issue and open a new issue describing an updated proposal when I have a chance. The idea will be to move more of the route matching logic into routes themselves, and allow configuring this sort of behavior on a route-by-route basis. This is inspired by @ryanflorence's examples with self-rendering routes.

[taion]

Folded into #3611.